leggedrobotics / elevation_mapping_cupy

Elevation Mapping on GPU.
MIT License
545 stars 116 forks source link

fix cell indexing #6

Closed rubengrandia closed 2 years ago

rubengrandia commented 2 years ago

trick is to move the half shift (width / 2) into the rounding operation to work for both even and odd numbered maps.

We also need to correct for the fact that the center of a cell is shifted by half the resolution when viewed from the center. That is where you get the (width - 1.0) / 2 from

rubengrandia commented 2 years ago

Old code: Screenshot from 2022-05-04 22-37-50

New code: Screenshot from 2022-05-04 22-35-54

lorenwel commented 2 years ago

Isn't the round the problem here? I think flooring with just int i = int((x - center) / resolution); should also be correct, no?

rubengrandia commented 2 years ago

Seems slightly different for even numbered maps: http://cpp.sh/6c3bd But maybe I didn't fully understand what you proposed

rubengrandia commented 2 years ago

But you are right that it should be possible to exploit the flooring of the int, instead of doing round manually. I think this one should work: return static_cast<int>((x - center) / resolution + static_cast<float>(width) / 2);

Basically removing the half shift again and then taking the floor instead of the round. I do think it is necessary to not divide width as an integer to be correct for both even and odd sized maps.

lorenwel commented 2 years ago

I meant more the problem with the original implementation. This is what I meant. So

int get_x_idx_floor(float x, float center, float resolution, int width) {
    return static_cast<int>((x - center) / resolution + width*0.5);
}
lorenwel commented 2 years ago

Ok, we came to the same conclusion :P I don't have a strong preference which one to use, I could imagine that integer casting is slightly faster. In any case, I would replace that / 2 with a * 0.5 :D

rubengrandia commented 2 years ago

I tested on the robot yesterday, looks fine. Let's wait for @mktk1117 's opinion. He can also run it on some larger offline dataset to double check.

mktk1117 commented 2 years ago

Tested from my side and it looks good.