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

Maybe need to pay attention to The Precision of Float Type... #1

Closed
ZesongLee opened this issue Oct 26, 2020 · 3 comments
Closed

Maybe need to pay attention to The Precision of Float Type... #1

ZesongLee opened this issue Oct 26, 2020 · 3 comments

Comments

@ZesongLee
Copy link

The Grid parameters are different...

2020-10-26 17:56:37.645472, frnn/frnn.py, frnn_grid_points, 272, r: tensor([0.1000, 0.1000], device='cuda:0')
2020-10-26 17:56:37.646062, frnn/frnn.py, forward, 62, r[i].item(): 0.10000000149011612, radius_cell_ratio: 2.0
2020-10-26 17:56:37.646082, frnn/frnn.py, forward, 63, cell_size: 0.05000000074505806
2020-10-26 17:56:37.646639, frnn/frnn.py, forward, 64, grid_params_cuda[i, 4:7]: tensor([20., 21., 20.], device='cuda:0')
2020-10-26 17:56:37.647305, frnn/frnn.py, forward, 62, r[i].item(): 0.10000000149011612, radius_cell_ratio: 2.0
2020-10-26 17:56:37.647324, frnn/frnn.py, forward, 63, cell_size: 0.05000000074505806
2020-10-26 17:56:37.648046, frnn/frnn.py, forward, 64, grid_params_cuda[i, 4:7]: tensor([20., 21., 20.], device='cuda:0')
@lxxue
Copy link
Owner

lxxue commented Nov 10, 2020

Sorry for the late reply, no idea why I didn't get the notification by email...

Thanks for the feedback. If I understand it correctly, you mean the grid size should be [20, 20, 20] but the float-point issue makes it to be [20, 21, 20]?

If that is the case, here is my considerations: in the general case, the size of the bounding box would not be the multiple of grid size / search radius. Then we don't have this kind of issues. Even if this happens, I guess both the computational overhead and the memory overhead is marginal.

Maybe I should change the data type of grid_params to be double? Do you have any suggestion?

@ZesongLee
Copy link
Author

I think the "round" operation seems to be more reasonable rather than "int" or "floor"

@lxxue
Copy link
Owner

lxxue commented Nov 20, 2020

That's a good idea. But if we round down, the search radius can be larger than cell_size * int_ratio, which means we need to search one more layer of grids (e.g. originally we search over a 5x5x5 grid, now with this round-down, we need to search over 7x7x7). According to my experiment results, this would be much slower if the search radius is not the exact multiple of the cell_size.

@lxxue lxxue closed this as completed Jun 29, 2021
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

2 participants