Skip to content

ENH: array comparison utils show calculations and indices of max errors #29395

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

ENH: array comparison utils show calculations and indices of max errors #29395

wants to merge 9 commits into from
+289 −146

Conversation

danra
Copy link
Contributor

@danra danra commented Jul 19, 2025 •
edited
Loading

Resolves #28827

  • Verbose assertion errors raised by testing.assert_allclose and testing.assert_array_XXX now show the calculation and the indices of the maximum absolute and relative differences among the violations.
  • Assertion errors raised by assert_array_almost_equal_nulp and assert_array_max_ulp, which have no verbose toggle, now show the indices of the maximum ULP difference.

The main complexity adding this feature was that one could not 'simply' replace max with argmax due to masking: the absolute differences are calculated after masking out infs and nans, and the relative differences are additionally calculated after masking out zero denominators.

Notes to reviewer:

  1. Based over MNT: Cleanup infs handling in np.testing assertion utilities #29321 which should be merged first.
  2. Best reviewed going over commits one-by-one. Main change is in the last commit, preceding commits lead up to it with minor preparations.

@danra danra force-pushed the max_errors_indices branch from 58a5820 to a967767 Compare July 21, 2025 04:29
@danra
Copy link
Contributor Author

danra commented Jul 21, 2025

Is the failing test maybe an instance of #29262 ?

@danra danra mentioned this pull request Jul 21, 2025
Closed
@ngoldbaum
Copy link
Member

ngoldbaum commented Jul 21, 2025 •
edited
Loading

It looks like pypy segfaulted on your PR. I don't think it's the same issue you linked to. Definitely a real problem but probably unrelated to this PR and safe for you to ignore.

Unfortunately the NumPy CI is sufficiently complicated that along with several heisenbugs bits and pieces regularly start failing for lots of reasons. Unless you're interested in helping out with a fix, it's usually safe to ignore. If you're unsure, just ask, and a maintainer should catch it if an error is real and it was ignored.

danra added a commit to danra/numpy that referenced this pull request Jul 28, 2025
@danra
DOC: Add release note for numpy#29395
4c65fff
ngoldbaum
ngoldbaum reviewed Jul 28, 2025
View reviewed changes
Copy link
Member

@ngoldbaum ngoldbaum left a comment

Choose a reason for hiding this comment

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

The pypy tests have segfaulted twice in the same spot now, so I think it's a real issue you're running into.

ping @mattip for pypy expertise.

doc/release/upcoming_changes/29395.improvement.rst
violations.
- Assertion errors raised by `assert_array_almost_equal_nulp` and
`assert_array_max_ulp` (which have no verbose toggle) now show the indices of
the maximum ULP difference.
Copy link
Member

Choose a reason for hiding this comment

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

the doc build fails with an error:

source/release/notes-towncrier.rst:72: WARNING: Bullet list ends without a blank line; unexpected unindent.

Maybe look at how the bullet lists are formatted in other release notes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After looking at other examples and adding a newline I still got the same error 🤷 . I just removed the bullets.

danra added a commit to danra/numpy that referenced this pull request Jul 29, 2025
@danra
DOC: Add release note for numpy#29395
74ef2ba
@danra danra force-pushed the max_errors_indices branch from 4c65fff to 74ef2ba Compare July 29, 2025 02:32
danra added a commit to danra/numpy that referenced this pull request Jul 29, 2025
@danra
DOC: Add release note for numpy#29395
60c73f9
@danra danra force-pushed the max_errors_indices branch from 74ef2ba to 60c73f9 Compare July 29, 2025 02:32
danra added 9 commits July 29, 2025 08:40
@danra
MNT: Remove redundant ravel
8ac6883 
Not needed since e6b68e5
@danra
MNT: Extract function to DRY
b010011
@danra
MNT: Extract variable towards upcoming change
188a135
@danra
MNT: nulp_diff now handles nans without adding extra dimension to result
28db159 
Noticed the extra dimension while adding the index of the max error to
assert_array_XXX_nulp output
@danra
MNT: Avoid unintentionally adding a dimension instead of masking
2e43b99
@danra
TST: Add missing digits to expected reported relative errors
3715639 
Noticed these while attempting to append more content to the
expected message, upon which the partial values started
causing the regex to stop matching.
@danra
MNT: Whitespace to minimize next diff
c7dfd4f
@danra
ENH: array comparison utils show calculations and indices of max errors
62271a5 
Resolves numpy#28827

- Verbose assertion errors raised by testing.assert_allclose and
  testing.assert_array_XXX now show the calculation and the indices of
  the maximum absolute and relative differences among the violations.
- Assertion errors raised by assert_array_almost_equal_nulp and
  assert_array_max_ulp, which have no verbose toggle, now show the
  indices of the maximum ULP difference.

The main complexity adding this feature was that one could not 'simply'
replace `max` with `argmax` due to masking: the absolute differences is
calculated after masking out infs and nans, and the relative differences
are additionally calculated after masking out zero denominators.
@danra
DOC: Add release note for numpy#29395
c3ababe
@danra danra force-pushed the max_errors_indices branch from 60c73f9 to c3ababe Compare July 29, 2025 15:41
@ngoldbaum
Copy link
Member

If I have some spare time I’ll try to reproduce this under gdb to figure out what the heck is happening.

@ngoldbaum
Copy link
Member

I can't reproduce the pypy crash on my mac.

@mattip
Copy link
Member

mattip commented Jul 31, 2025

I could reproduce it on linux, adding some gc.collect() at the end of tests seems to clear it. Not worth holding up this PR for the problem. If it becomes annoying, we can disable pypy in CI, anyway we will soon drop python3.11.

@danra
Copy link
Contributor Author

danra commented Aug 12, 2025

Can someone please review?

@mattip
Copy link
Member

mattip commented Aug 12, 2025

Is the intent to make this

>>> assert_almost_equal(np.array([1.0,2.3333333333333]),
    ...                     np.array([1.0,2.33333334]), decimal=9)
    Traceback (most recent call last):
        ...
    AssertionError:
    Arrays are not almost equal to 9 decimals

    Mismatched elements: 1 / 2 (50%)
    Mismatch at index:
     [1]: 2.3333333333333 (ACTUAL), 2.33333334 (DESIRED)
    Max absolute difference among violations: 6.66669964e-09
    Max relative difference among violations: 2.85715698e-09

     ACTUAL: array([1.         , 2.333333333])
     DESIRED: array([1.        , 2.33333334])

into this (note the additional lines marked with <-----)

>>> assert_almost_equal(np.array([1.0,2.3333333333333]),
    ...                     np.array([1.0,2.33333334]), decimal=9)
    Traceback (most recent call last):
        ...
    AssertionError:
    Arrays are not almost equal to 9 decimals

    Mismatched elements: 1 / 2 (50%)
    Mismatch at index:
     [1]: 2.3333333333333 (ACTUAL), 2.33333334 (DESIRED)
    Max absolute difference among violations: 6.66669964e-09
     |2.33333333 - 2.33333334| at index [1]                <--------------
    Max relative difference among violations: 2.85715698e-09
     |2.33333333 - 2.33333334| / |2.33333334| at index [1] <-------------- 

     ACTUAL: array([1.         , 2.333333333])
     DESIRED: array([1.        , 2.33333334])

? If I understand correctly, I think it would be better to reformat the current information rather than to repeat it yet again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ngoldbaum ngoldbaum ngoldbaum left review comments

Assignees
No one assigned
Labels
01 - Enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

ENH: assert_allclose show indices and values of maximum differences on failure
3 participants
@danra @ngoldbaum @mattip