Skip to content
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

Adding matrix R in UKF covariance update in correct (711) #718

Open
wants to merge 2 commits into
base: noetic-devel
Choose a base branch
from

Conversation

lkdo
Copy link

@lkdo lkdo commented Jan 17, 2022

No description provided.

@lkdo lkdo closed this Jan 17, 2022
@lkdo lkdo reopened this Jan 28, 2022
@lkdo
Copy link
Author

lkdo commented Jan 28, 2022

@ayrton04
Hi,

These changes trigger a regression test failure,
but since this is a modification of the actual numerics of the filter,
I belive this is ok, and that the regression test must be modified.

Please let me know how to proceed.

best regards,

@ayrton04
Copy link
Collaborator

Hmmm, I'd want to track down why it's different. If you look at the failing tests, you can see that the error between what we're expecting from the integration and what we're seeing has grown considerably:

https://build.ros.org/job/Npr__robot_localization__ubuntu_focal_amd64/68/testReport/junit/(root)/InterfacesTest/ImuDifferentialIO/

https://github.com/lkdo/robot_localization/blob/e6d56e024b9d5b02d562dcb6ddf8606f53726da1/test/test_ukf_localization_node_interfaces.cpp#L1061-L1063

It may be the case that the changes I made to get these tests to work after I updated the UKF need to be put back.

lkdo@0fe3097#diff-7f0519215d4ab3adad9e1164ae9d97e87a8d700152dbfe4bd85108301db448b7R49-R54

lkdo@0fe3097#diff-c484b9dfe684344cfebf71c5b5619f237c50e872c2f520d5300698333efc5426R986-R997

Note that the same limits are used for the EKF.

https://github.com/lkdo/robot_localization/blob/e6d56e024b9d5b02d562dcb6ddf8606f53726da1/test/test_ekf_localization_node_interfaces.cpp#L1057-L1059

So we definitely need to make sure we're not doing something erroneous.

@ayrton04
Copy link
Collaborator

I'm having a hard time finding our conversation about NaN values creeping into the UKF estimate. Did you ever manage to re-create the same issue with the upstream code?

@ayrton04
Copy link
Collaborator

ayrton04 commented May 3, 2022

Just checking in on this ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants