-
Notifications
You must be signed in to change notification settings - Fork 15
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
Barycentric interpolation of science sensor data #110
Conversation
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
…ces, match data accordingly; concatenate 2 input arrays into 1; correct inputs to trilinear interpolation Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
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.
I like the simpler approach here. I think that we can improve the logic on the K nearest neighbors by potentially researching/sampling if we notice that we get a lopsided sample. Aka they're all on one side, reslice the data and search for the nearest neighbor on the other side. Such that we guarantee that the tetrahedron surrounds the point. Otherwise the math is invalid. And if we're doing that logic we could probably avoid the 2d reduced case.
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
…ive case for interpolation Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
0d0b14d makes the math more rigorous, in checking the matrix T for zero rows and handling degenerative cases accordingly (see sample output below). 0d0b14d also fixes a logic error in the for-loop over sensors. There had to be two separate loops, the first one is for positioning, for which only one sensor needs to be checked. The second one is for interpolating the data, which all sensors need to go through, because the input is not just the position but each sensor's individual data. This is an example 1D case that is now handled, previously it gave NaNs because T has 2 rows of zeros.
|
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
…es within Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
Signed-off-by: Mabel Zhang <mabel@openrobotics.org>
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.
I've read through the code and seen it running. I'd really like to have unit tests for the interpolation in isolation so that we can be confident in it outside the context of this plugin. But after looking into that there's no good way for us to instrument into the private implementation class.
I'd suggest that we plan in the future to refactor that out in to a standalone library which can be unit tested. But for now we merge this so we can build on top of it. It will be used every run and there's not many branch points so it should be obvious if things aren't working well. And we can defer filling in coverage testing.
Thank you for the review! Yeah, I hadn't thought about the class being private. As an intermediate to having a standalone library, we might be able to still add tests like with other plugins by checking the sensor output, which should be the interpolation result given the robot location and some known data. I think that should be doable in the test infrastructure. |
Only test failing is Will merge this for now and add improvements from manual tests, and add automated tests in new PRs. |
Supersedes #83
Long story short, I finished implementing trilinear interpolation (in another branch,
mabelzhang/trilinear_interp
), which took a lot of sanity checks to make sure the inputs are 8 vertices of a prism, a lot of sorting the data to make sure which corner lies where and use the correct data corresponding to its corner, blah blah... in the end, the inputs didn't even meet the assumptions.So, I've switched to barycentric interpolation instead. It took a fraction of the time of trilinear interpolation to implement, as in hours as opposed to days and weeks. Most importantly, it works.
Let's just use barycentric, at least for now.
Why barycentric is better than trilinear:
This is a big deal.
The 8 points being structured takes 4 kNN searches to get 2 z-slices from the unstructured point cloud. Searching for the 2nd z-slice requires removing the 1st z-slice, which messes up the indices, so you need to manually keep track in order to retrieve the correct values later for interpolation.
The prism assumption takes a lot of tedious code to verify inputs, and to match the data to the right corners.
I didn't throw away the trilinear interpolation code in the other branch, in case we want to return to it. If we don't, some weeks later it'll be trashed.
Ignore the many commits (artifacts from adding barycentric on top of trilinear, which is a branch that's been around for a while and I kept merging everything back into). We'll squash and merge anyway.
Sample output
2D case:
The default robot position of 0 0 0 enacts the 2D case, a special case when matrix T has 3rd row all zeros.
Then, the overloaded 2D function taking
Eigen::Vector2f
is called.Check that the last line is reasonable - result is same as first vertex, because first vertex is at 0 0 0.
3D case:
Change robot pose in
buoyant_tethys.sdf
, so that the z is between two z slices:Check that the last line is reasonable, somewhere between the vertices.