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

Small bugs in velocity map saturation and heuristics #13

Open
Magnus-Noren opened this issue Mar 8, 2024 · 2 comments
Open

Small bugs in velocity map saturation and heuristics #13

Magnus-Noren opened this issue Mar 8, 2024 · 2 comments

Comments

@Magnus-Noren
Copy link

Magnus-Noren commented Mar 8, 2024

Hello,

Thank you for making this public. I find the library very useful in my current research.

I found a couple of small bugs when testing the code.

  1. I found that the velocity map wasn't saturated at the correct distance when setting maxDistance != -1. This has two causes. First, the velocity shouldn't first be normalized (by dividing it by maxValue), when doing the saturation step. Secondly, the division by getLeafSize() shouldn't be there, since the leaf size is already accounted for in the Eikonal solver. With these changes, you get the saturation at the correct distance in real units, regardless of leaf size.
  2. Also related to leaf size, when using heuristics, the setHeuristicTime forgets to take the leaf size into account, which leads to strange behaviour when leafSize != 1. I think the least intrusive fix is to multiply the return value by getLeafSize() in getPrecomputedDistance: return distances_[idx_dist] * grid_->getLeafSize();.

I made these changes on my end, so I'm not asking you to do anything, I just thought you'd might like to know. Once again, thanks!

@jvgomez
Copy link
Owner

jvgomez commented Mar 8, 2024

Hi!

Thanks for the nice words and for reporting the issues!

I am not actively maintaining the library, but if you make a pull request we can merge it :)

@Magnus-Noren
Copy link
Author

Magnus-Noren commented Mar 8, 2024

Hmmm, maybe. I'm really a noob when it comes to git-related stuff, and don't know if I've ever done a pull request. Also, I contaminated the repo with some of my own stuff, for example added pybind11 as a submodule in order to make Python bindings for the parts of the library I'm using, and modified the CMakeLists.txt for that purpose. But I guess if I add those things to the gitignore, it should work?
Basically, the only (small) changes I did to the original files are in fmm.hpp (in getPrecalculatedDistance) and fm2.hpp (in computeVelocitiesMap).

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