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

Handle new warnings introduced in NumPy 1.24 #6637

Merged
merged 9 commits into from Nov 29, 2022

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Nov 28, 2022

The pre-release test case on CI fails due to a few new warnings introduced in NumPy 1.24 (example of a recent test log). (Our pre-release test CI job treats warnings as errors via env var SKIMAGE_TEST_STRICT_WARNINGS_GLOBAL=1)

The changes here should resolve these failures.

This warning appears with NumPy>=1.24 where np.int0 has been deprecated
This warning appears with NumPy>=1.24 where np.bool8 has been deprecated
@grlee77 grlee77 added 🤖 type: Infrastructure CI, packaging, tools and automation 🩹 type: Bug fix Fixes unexpected or incorrect behavior labels Nov 28, 2022
@grlee77 grlee77 added this to the 0.20 milestone Nov 28, 2022
@grlee77
Copy link
Contributor Author

grlee77 commented Nov 28, 2022

The fix for Cascade in f25ae85 does not seem like the correct solution as it will break again with an error once the deprecation cycle completes. I am not familiar with that code so would like to leave a proper fix to a separate PR. I will open an issue for it. The reported warning from the log is:

FAILED feature/tests/test_cascade.py::test_detector_astronaut - DeprecationWarning: NumPy will stop allowing conversion of out-of-bound Python integers to integer arrays.  The conversion of -67130709 to uint32 will fail in the future.
For the old behavior, usually:
    np.array(value).astype(dtype)`
will give the desired result (the cast overflows).

@grlee77
Copy link
Contributor Author

grlee77 commented Nov 28, 2022

Three failures remaining in test_ciede2000_dE that are a bit more concerning as at seems the tolerance to pass the test has changed.

FAILED color/tests/test_delta_e.py::test_ciede2000_dE[float64-0] - AssertionError: 
Not equal to tolerance rtol=0.0001, atol=0

Mismatched elements: 1 / 34 (2.94%)
Max absolute difference: 0.04027009
Max relative difference: 0.00560927
 x: array([ 2.04246 ,  2.86151 ,  3.441191,  0.999999,  1.000005,  1.000013,
        2.366859,  2.366859,  7.179172,  7.21947 ,  7.219472,  7.219474,
        4.804522,  4.804525,  4.746071,  4.306482, 27.149231, 22.897692,...
 y: array([ 2.0425,  2.8615,  3.4412,  1.    ,  1.    ,  1.    ,  2.3669,
        2.3669,  7.1792,  7.1792,  7.2195,  7.2195,  4.8045,  4.8045,
        4.7461,  4.3065, 27.1492, 22.8977, 31.903 , 19.4535,  1.    ,...
FAILED color/tests/test_delta_e.py::test_ciede2000_dE[float64-1] - AssertionError: 
Not equal to tolerance rtol=0.0001, atol=0

Mismatched elements: 1 / 34 (2.94%)
Max absolute difference: 0.04027009
Max relative difference: 0.00560927
 x: array([ 2.04246 ,  2.86151 ,  3.441191,  0.999999,  1.000005,  1.000013,
        2.366859,  2.366859,  7.179172,  7.21947 ,  7.219472,  7.219474,
        4.804522,  4.804525,  4.746071,  4.306482, 27.149231, 22.897692,...
 y: array([ 2.0425,  2.8615,  3.4412,  1.    ,  1.    ,  1.    ,  2.3669,
        2.3669,  7.1792,  7.1792,  7.2195,  7.2195,  4.8045,  4.8045,
        4.7461,  4.3065, 27.1492, 22.8977, 31.903 , 19.4535,  1.    ,...
FAILED color/tests/test_delta_e.py::test_ciede2000_dE[float64--1] - AssertionError: 
Not equal to tolerance rtol=0.0001, atol=0

Mismatched elements: 1 / 34 (2.94%)
Max absolute difference: 0.04027009
Max relative difference: 0.00560927
 x: array([ 2.04246 ,  2.86151 ,  3.441191,  0.999999,  1.000005,  1.000013,
        2.366859,  2.366859,  7.179172,  7.21947 ,  7.219472,  7.219474,
        4.804522,  4.804525,  4.746071,  4.306482, 27.149231, 22.897692,...
 y: array([ 2.0425,  2.8615,  3.4412,  1.    ,  1.    ,  1.    ,  2.3669,
        2.3669,  7.1792,  7.1792,  7.2195,  7.2195,  4.8045,  4.8045,
        4.7461,  4.3065, 27.1492, 22.8977, 31.903 , 19.4535,  1.    ,...
====== 3 failed, 8306 passed, 30 skipped, 5 warnings in 340.68s (0:05:40) ======
Error: Process completed with exit code 1.

@grlee77
Copy link
Contributor Author

grlee77 commented Nov 28, 2022

Regarding the relatively low rtolused in test_ciede2000_dE, I think it is likely there is substantial numerical error in the term:
np.sqrt(c7 / (c7 + 25 ** 7)) where c7 might have very small values while 25**7 is large, so there may be substantial floating point error.
I don't know why the situation would have changed for the double precision test case only in NumPy 1.24rc1 vs. older releases, though.

updated the comment as I am not sure what term is the primary contributor to the numerical error and don't think it is the one I originally mentioned

@@ -109,7 +109,7 @@ def test_copy():

def test_bool():
img_ = np.zeros((10, 10), bool)
img8 = np.zeros((10, 10), np.bool8)
img8 = np.zeros((10, 10), np.bool_)
Copy link
Member

Choose a reason for hiding this comment

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

Should np.bool_ just be bool?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think they are all equivalent. They do have different entries here, though:

dtype_range = {bool: (False, True),
np.bool_: (False, True),
np.bool8: (False, True),

so I think that is why there was both bool and np.bool8 separately here previously. I just changed bool8 to bool_ since that one is not deprecated.

grlee77 and others added 3 commits November 28, 2022 17:36
Co-authored-by: Mark Harfouche <mark.harfouche@gmail.com>
Co-authored-by: Brett M. Morris <morrisbrettm@gmail.com>
@grlee77
Copy link
Contributor Author

grlee77 commented Nov 29, 2022

@bmorris3, I pulled your Cascade fix from #6633 to here in 4b21720. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🩹 type: Bug fix Fixes unexpected or incorrect behavior 🤖 type: Infrastructure CI, packaging, tools and automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants