Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

questions about custom_kernels.py #44

Open
GuaguaguaLiu opened this issue Feb 12, 2023 · 4 comments
Open

questions about custom_kernels.py #44

GuaguaguaLiu opened this issue Feb 12, 2023 · 4 comments

Comments

@GuaguaguaLiu
Copy link

GuaguaguaLiu commented Feb 12, 2023

Hello, Miki, thanks for your great job and open source sharing.

  1. In line 270 of elevation_mapping.py, when call the function dilation_filter_kernel, why are the inputs self. elevation_ Map [5] and self. elevation_ map[2] + self.elevation_ map [6] instead of self. elevation_ map [0] and self. elevation_ map[2] ? Notice self.elevation_ map[0],self.elevation_ map [2] and self.elevation_ map [6] represent the current height, is_ valid value and is_ upper_ bound value of the grid, respectively. What is the specific meaning of self. elevation_ map[2] + self.elevation_ map [6] ?

  2. In line 284 of elevation_mapping.py, when call update_normal function, why is the input self.traversability_input instead of self.elevation_map[0] ? What is the meaning of self.normal_map calculated in this way?

  3. In line 192 of custom_kernels.py: map[get_map_idx (idx, 5)]=new_ h;, Does this means that it adopts the new_h calculated with the last measurement point which falls in this grid? Why?

  4. In lines 178 and 248 of custom_kernels.py, why num_ points take different values, namely newmap[get_map_idx (idx, 4)] and newmap[get_map_idx (nidx, 3)] ? And what the meaning of ${enable_edge_shaped}?

  5. Note that some CUDA kernels (e.g., add_points_kernel) are defined. It seems that the codes of these kernels are not easy to debug. Do you have any recommended debugging methods?

I am looking forward to your reply, thank you!

@mktk1117
Copy link
Collaborator

Hi thank you for checking out the code in details.

  1. It is to have a traversability value also in the upper bounded area. The plus operation is to take the or of elevation_map[2] and elevation_map[6]
  2. This is because to calculate normal, it is using the surrounding map height. To prevent weird value near the edge, we use the dilated layer. Since it is already calculated as traversability_input, we reused it.
  3. this value will be anyway not used since upper_bound is effective in the invalid cell. need to check why this was added.
  4. The first num_points (idx, 4) is the number of all points fall into the same cell. Second num_points (idx, 3) is the points in the flat region which is used for calculating the height drift. In the ray-casting, we used this value to prevent removing flat area.
  5. It is indeed tricky to debug. I was providing some array to substitute some intermediate value to check. I also want to know a better way to debug.

Hope these could help your understanding.

@GuaguaguaLiu
Copy link
Author

GuaguaguaLiu commented Feb 15, 2023

Thank you very much for your reply. There are still some unclear points about the first and third points.

From the code, we can see the upper bound layer (i.e., elevation_map[5]) is used as the input of the dilation_filter_kernel to calculate the dilated layer, and further calculate the traversability layer and normal map. but you mentioned that

this value will be anyway not used since upper_bound is effective in the invalid cell.

What is the meaning of this?

On the other hand, it can be seen that the assignment of the upper bound layer (i.e., elevation_map[5]) comes from two sources:

  1. In line 192 of custom_kernels.py, map[get_map_idx (idx, 5)]=new_ h. but the puzzle here is that, why the result calculated from the last measurement point is taken as the upper bound value rather than using the average result of all measurement points. Based on you mentioned that the value will not be used, I have tried to remove this line. However, it will lead to the error result of the traversability layer.
  2. When visibility cleanup is enabled, the upper bound values of some grids will be updated.

Intuitively, why should we use the upper bound layer (i.e., elevation_map[5]) instead of the elevation layer (i.e., elevation_map[0]) as the input of the dilation_filter_kernel and further calculate the traversability layer and normal map?

@lorenwel
Copy link
Member

Thank you very much for your reply. There are still some unclear points about the first and third points.

From the code, we can see the upper bound layer (i.e., elevation_map[5]) is used as the input of the dilation_filter_kernel to calculate the dilated layer, and further calculate the traversability layer and normal map. but you mentioned that

this value will be anyway not used since upper_bound is effective in the invalid cell.

What is the meaning of this?

The upper_bound layer has the same value as the elevation layer, in every cell where we have a valid elevation value. Therefore, we update upper_bound with the same new height value new_h. What Takahiro probably meant is that the upper_bound value doesn't really make sense, if we also have an elevation value, since that one should be used. For convenience, we still fill the upper_bound value, regardless.

On the other hand, it can be seen that the assignment of the upper bound layer (i.e., elevation_map[5]) comes from two sources:

  1. In line 192 of custom_kernels.py, map[get_map_idx (idx, 5)]=new_ h. but the puzzle here is that, why the result calculated from the last measurement point is taken as the upper bound value rather than using the average result of all measurement points. Based on you mentioned that the value will not be used, I have tried to remove this line. However, it will lead to the error result of the traversability layer.

As mentioned above, this line makes sure that the upper_bound layer has the same values as the elevation layer wherever the elevation layer is valid. Since elevation represents the actual terrain height, the upper_bound must be the same.

  1. When visibility cleanup is enabled, the upper bound values of some grids will be updated.

This is the actual ray-casting operation to update the upper_bound layer where we do not have any valid elevation. Line 232 is only reached if there is no valid elevation (Check in line 230). Since there is no measurement in this cell, but a measurement ray passes through the cell, we know that the terrain cannot be above this height. Otherwise, the ray would have observed the terrain. Therefore, we can set nz as the upper_bound value. For the upper_bound layer, filtering or averaging of measurements does not make sense, since it must be the result of a min(ray_height) operation.

Intuitively, why should we use the upper bound layer (i.e., elevation_map[5]) instead of the elevation layer (i.e., elevation_map[0]) as the input of the dilation_filter_kernel and further calculate the traversability layer and normal map?

As you can see, the upper_bound layer is equivalent to the elevation layer, wherever the elevation is valid, but also has an estimate of the maximum terrain height in invalid cells. Therefore, it has strictly more or equal information as the elevation layer. Therefore, we use this layer as input to the dilation kernel and to compute traversability.

Note that in the default configuration, the traversability is only considered valid where elevation is valid. Since upper_bound == elevation in these regions, the traversability values computed here are the same in almost all cases, whether we would use elevation or upper_bound as their basis. However, using the upper_bound layer, we also have traversability values for all its cells, if a user wants to use upper_bound.

@GuaguaguaLiu
Copy link
Author

GuaguaguaLiu commented Feb 27, 2023

Thank you very much for your reply!

You have pointed out that the upper_bound layer should have the same value as the elevation layer in every cell where we have a valid elevation value. I quite agree with that.

The upper_bound layer has the same value as the elevation layer, in every cell where we have a valid elevation value.

However, according to the evaluation results and reading the code, I find that this is not the case, that is, the values of the upper_bound layer and the elevation layer are not the same, even in the cell where we have a valid elevation value.

In particular, I have attached the elevation map results after a point cloud processing in the attachment, including the elevation layer (elevation_map[0]), is_valid layer(elevation_map[2]) and upper_bound layer(elevation_map[5]). For example, at the index of (131, 3), the is_valid value is 1, representing this grid is valid. However, the values of the elevation and upper_bound are -1.15468 and -1.1552, respectively, which are different. There are other examples to illustrate this point.

Moreover, from the code, map[get_map_idx (idx, 5)]=new_ h in line 192 of custom_kernels.py, you say

As mentioned above, this line makes sure that the upper_bound layer has the same values as the elevation layer wherever the elevation layer is valid.

However, I don't think it can actually guarantee that the upper_bound layer has the same values as the elevation layer. The reason is that this kernel function add_points_kernel is iterated through every point in current measurement, which causes the result of the last point to overwrite the previous result, i.e., only the new_h of the last point will be saved in this grid. Instead, for the elevation layer, we can see the line 184 of custom_kernels.py: atomicAdd(&newmap[get_map_idx(idx, 0)], new_h). This means that it actually saves all the results of each point by summing, and then obtains the elevation value of the grid by averaging it. This is different from the calculation of the upper_bound value mentioned above.

Looking forward to your reply, thank you very much!

elevation_layer.xlsx
is_valid_layer.xlsx
upper_bound_layer.xlsx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants