Skip to content

fix(test_from_matrix): assume max precision - #36

Open
MarcoMiretti wants to merge 1 commit into
satellogic:masterfrom
MarcoMiretti:fix-test-from-matrix
Open

fix(test_from_matrix): assume max precision#36
MarcoMiretti wants to merge 1 commit into
satellogic:masterfrom
MarcoMiretti:fix-test-from-matrix

Conversation

@MarcoMiretti

Copy link
Copy Markdown

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
Copy Markdown
Author

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

@coveralls

coveralls commented Sep 8, 2020

Copy link
Copy Markdown

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
Copy Markdown
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