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

Compare regression test quaternions with delta-quat tolerance check #288

Merged
merged 3 commits into from
Jun 26, 2023

Conversation

jeanconn
Copy link
Contributor

@jeanconn jeanconn commented Jun 24, 2023

Description

Compare regression test quaternions with delta-quat tolerance check instead of == test.

This fixes the issue that the calculated quaternions on fido present insignificant numeric differences from the same quaternions calculated on kady.

Interface impacts

Testing

Unit tests

  • Linux on fido at ska3-matlab 2023.4rc6

Independent check of unit tests by @taldcroft

  • Mac

Functional tests

No functional testing.

kadi/commands/tests/test_commands.py Outdated Show resolved Hide resolved
kadi/commands/tests/test_commands.py Outdated Show resolved Hide resolved
kadi/commands/tests/test_commands.py Outdated Show resolved Hide resolved
@jeanconn jeanconn changed the title WIP: Remove test with att comparison of nsm quats that breaks on fido Compare regression test quaternions with delta-quat tolerance check Jun 26, 2023
@jeanconn jeanconn requested a review from taldcroft June 26, 2023 17:18
Copy link
Member

@taldcroft taldcroft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, assuming that you ran tests on the latest commit in this PR.

@jeanconn
Copy link
Contributor Author

Yes - that was what I tested before moving this from non-WIP status and asking for review.

@jeanconn jeanconn merged commit a9c5db8 into master Jun 26, 2023
@jeanconn
Copy link
Contributor Author

Honestly, I hadn't run the tests on kady, so I'll leave that as an exercise for integration testing.

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.

2 participants