-
Notifications
You must be signed in to change notification settings - Fork 206
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
Bugfix: correctly calculating free cell #10
base: master
Are you sure you want to change the base?
Conversation
@dabarsch thanks for the pr! I don't really have the time, nor the internal understanding of gmapping to review your change. Maybe @vrabaud or @mikeferguson will have time to look at it, but you may have to be patient since I know they're extremely busy as well. We'll get to it as soon as we're able. |
@mikeferguson , I am trying to quantify the changes: would the entropy be the right measure to look at on some datasets ? |
ok, I'd like some feedback here: overall, it seems the entropy is lower on the test datasets. Which is good and seems to show your fix is good. Now, it makes things slower (like 10/20%). @dabarsch , any feedback on that ? Also, could you please respect the tabs in your commits ? (yes, I knows, it should be spaces but let's respect the code). Also, could you please add comments about what those structures are: I kindof understand your fix but I am not completely sure. Please forgive my ignorance, I am just a maintainer :) |
this entropy is bs, it's only based on weights: after re-sampling, it equals to the same value for any case .... We need a better measure of quality here. |
Or a flag. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dabarsch could you create new PR against melodic-devel
branch? we have switched from master
to melodic-branch
when we changed licence to BSD
The free cell is calculated somewhere off the map. Since unvisited cells return -1 which is lower than the fullness threshold it wasn't dramatically influencing the scoring. Moreover in icpStep and score the factor map.getDelta() was multiplied twice.