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

DM-43855: Rework error reporting for assertImagesAlmostEqual #734

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

parejkoj
Copy link
Contributor

Just reporting the maximum absolute difference does not tell you where np.allclose() was failing, because it uses a relative+absolute comparison.

@parejkoj parejkoj force-pushed the tickets/DM-43855 branch 2 times, most recently from c04302f to 0c057f6 Compare May 17, 2024 23:41
@parejkoj parejkoj force-pushed the tickets/DM-43855 branch from 0c057f6 to 16f0c56 Compare May 31, 2024 09:04
@parejkoj
Copy link
Contributor Author

parejkoj commented Jun 1, 2024

Example error message from this change:

E   AssertionError: variance planes differ: 191989 pixels failing np.allclose(), worst is: |0.5726712942123413 - 1.0| = 0.4273287057876587 > 0.010009999386966228 (rtol*abs(image2)+atol with rtol=1e-05, atol=0.01) at position (150, 154), and maximum absolute error: |0.5726712942123413 - 1.0| = 0.4273287057876587 at position (150, 154).

@parejkoj parejkoj force-pushed the tickets/DM-43855 branch from 16f0c56 to 222e16c Compare June 1, 2024 01:32
maxAbsInd = np.where(errArr == maxErr)
maxAbsTuple = (maxAbsInd[1][0], maxAbsInd[0][0])
# NOTE: use the second image, because the numpy test is:
# (atol + rtol * absolute(b))
Copy link
Member

Choose a reason for hiding this comment

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

I trust this is all right, but this comment and the commit message aren't quite enough for me to easily grasp what it is that allclose is doing. Adding min and/or max to this expression and an inequality might help.

parejkoj added 2 commits June 3, 2024 12:45
Just reporting the maximum absolute difference does not tell you where
np.allclose() was failing, because it uses a relative+absolute
comparison.

Fix test of error message format for unsigned case.
@parejkoj parejkoj force-pushed the tickets/DM-43855 branch from 222e16c to bb3a0c3 Compare June 3, 2024 19:45
@parejkoj parejkoj merged commit adfd92a into main Jun 3, 2024
2 checks passed
@parejkoj parejkoj deleted the tickets/DM-43855 branch June 3, 2024 21:55
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