ros-perception / openslam_gmapping

218 stars 206 forks source link

Bugfix: correctly calculating free cell #10

Open dabarsch opened 9 years ago

dabarsch commented 9 years ago

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.

wjwwood commented 9 years ago

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

vrabaud commented 9 years ago

@mikeferguson , I am trying to quantify the changes: would the entropy be the right measure to look at on some datasets ?

vrabaud commented 9 years ago

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

vrabaud commented 9 years ago

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.

topin89 commented 4 years ago

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.