-
Notifications
You must be signed in to change notification settings - Fork 902
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
Fix UKF Euler angle handling #778
base: noetic-devel
Are you sure you want to change the base?
Conversation
1abfa34
to
710d68e
Compare
d6a85fd
to
02f1429
Compare
So, I've finally got to testing this. It's definitely better as now at least the UKF node doesn't hang. However, the state estimation still has some problem compared to EKF. I attach two bags, one running the EKF node, one running the UKF node, the exact same settings. I drove the robot forward, turn right, backward, turn left, backward, forward, backward. These motions are quite ok in the EKF bag, but the UKF estimate is quite bad - backwards motion is sometimes ignored (the first one), or interpreted as forward motion (the other ones). I haven't played with any of the UKF-specific variables. I use this config for integrating the 2D odometry:
Looking at the twist parts of the odometry messages, they seem to match the 2D odometry input. So only the position estimate is wrong. |
Thanks for the new data. This looks like a numerical issue with the UKF. For reference, for a state with dimension The issue here is the weights themselves. The weights must sum to The original issue when this ticket was created was that the negative weight was affecting the decomposed Euler angles and changing the values we get when we eventually move them back into Euler space via I'll focus on just the
Here are the values for
The values are all around There are other strategies for setting the weights. I'll try some simpler ones. |
Addresses #777.
The main issue is that UKF weights can be negative. That's fine in general; the UKF only cares whether the weights sum to 1. What we typically see is that the "mean" state has a very large negative weight, and then each of the lambda points gets a small positive weight.
Unfortunately, our use of Euler angles bites us (again) here, because the way we have to do weighted averaging requires us to decompose each Euler angle into
x
andy
components usingcos
andsin
, respectively. We then weight those values, sum them, and then recover the final weighted average viaatan2
. And therein lies the problem.atan2
will return the following values (image taken from Wikipedia):So if the UKF weights end up changing the sign of the summed
x
component of the Euler angle decomposition, we end up completely changing the recovered angle by +/- pi. That's obviously a huge problem that rears its head whenever the covariance on those values gets too large.This is another case where a different representation of the angles would be the best way to solve this problem. But I am not going to make changes that fundamental at this stage in this package's life cycle, so I went another route. For angles, I think the signs of the weights can all be positive. All we care about is that the first sample is represented much more heavily than the others. It seems to solve the problem stated in #777, and it also still passes all tests. I've verified the behavior on the test bags visually as well.