search

jvgomez / fast_methods

N-Dimensional Fast Methods: Fast Marching, Fast Sweeping, Group Marching, Fast Iterative, etc.
http://jvgomez.github.io/fast_methods/
GNU General Public License v3.0
167 stars 57 forks source link

Small bugs in velocity map saturation and heuristics #13

Open Magnus-Noren opened 5 months ago

Magnus-Noren commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

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).