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

fix(test_from_matrix): assume max precision #36

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

MarcoMiretti
Copy link

When converting to matrix back and forth, coordinates appear to be
rounded to a precision of 1e-8. This causes sporadic failures in said
test.

For this test it should be assumed that the coordinates of arr can be
described with said precision.

Solves issue #30

Example of what i think is causing the problem:

>>> q = Quaternion(*arr)
>>> q
Quaternion(0.9694471267866732, 0.0, 8.711074176117386e-16, 0.2453003635648832)
>>> m = q.matrix
>>> m
array([[ 8.79655463e-01,  4.75611465e-01, -1.68898517e-15],
       [-4.75611465e-01,  8.79655463e-01,  4.27365932e-16],
       [ 1.68898517e-15,  4.27365932e-16,  1.00000000e+00]])
>>> q.from_matrix(m)
Quaternion(0.9694471267866732, 0.0, 1.0536712127723509e-08, 0.24530036356488336)

@MarcoMiretti
Copy link
Author

Hmm. i guess something funny is happening with precision in python 3.4 🤔

@coveralls
Copy link

coveralls commented Sep 8, 2020

Coverage Status

Coverage remained the same at 98.467% when pulling b911ed2 on MarcoMiretti:fix-test-from-matrix into 649d47b on satellogic:master.

@MarcoMiretti MarcoMiretti force-pushed the fix-test-from-matrix branch 3 times, most recently from a66d248 to 8854964 Compare September 9, 2020 14:31
When converting to matrix back and forth, coordinates appear to be
rounded to a precision of 1e-8. This causes sporadic failures in said
test.

Routd the arr coordiantes to DEFAULT_TOLERANCE.
@MarcoMiretti
Copy link
Author

Hmm. i guess something funny is happening with precision in python 3.4 thinking

Fixed it changing the approach. Instead of assuming that the values are rounded, just round them at the beginning.

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