Skip to content

TST: Add future dependency tests as a weekly CI job #21634

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

Merged
merged 1 commit into from
Feb 12, 2022

Conversation

greglucas
Copy link
Contributor

PR Summary

Test future numpy versions with Matplotlib to see if anything needs to be done in the future to address deprecations
or other pending changes.

  • Currently will run once per week at 03:47 UTC on Saturdays.
  • Only runs on Linux and for 3.9/3.10 currently as this is mostly just a smoke test for future issues.
  • I think it will upload an issue on failure to notify the repo, but that is untested. Perhaps there is a better way here that someone knows about.

Link to a run on my branch (it should only run on matplotlib/matplotlib now)
https://github.com/greglucas/matplotlib/runs/4204821100?check_suite_focus=true

3.9 + numpy-1.22.dev failures

FAILED lib/matplotlib/tests/test_units.py::test_plot_masked_units[png] - matp...
= 1 failed, 8352 passed, 148 skipped, 13 xfailed, 4 xpassed in 537.93s (0:08:57) =

3.10 + numpy-1.22.dev failures

FAILED lib/matplotlib/tests/test_axes.py::test_errorbar[png] - matplotlib.tes...
FAILED lib/matplotlib/tests/test_axes.py::test_errorbar[svg] - matplotlib.tes...
FAILED lib/matplotlib/tests/test_axes.py::test_errorbar[pdf] - matplotlib.tes...
FAILED lib/matplotlib/tests/test_backends_interactive.py::test_webagg - Asser...
FAILED lib/matplotlib/tests/test_units.py::test_plot_masked_units[png] - matp...
FAILED lib/matplotlib/tests/test_streamplot.py::test_direction[png] - matplot...
FAILED lib/mpl_toolkits/tests/test_axisartist_grid_helper_curvelinear.py::test_axis_direction[png]
FAILED lib/mpl_toolkits/tests/test_mplot3d.py::test_trisurf3d[png] - matplotl...
FAILED lib/mpl_toolkits/tests/test_mplot3d.py::test_stem3d[png] - matplotlib....
= 9 failed, 8344 passed, 148 skipped, 13 xfailed, 4 xpassed, 4 warnings in 414.94s (0:06:54) =

Additional things to consider

Perhaps we should also consider adding a nightly cibuildwheel to that repository for others to test out Matplotlib and report back to us easier?

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • New features are documented, with examples if plot related.
  • New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).

run: |
xvfb-run -a python -mpytest -raR -n auto \
--maxfail=50 --timeout=300 --durations=25 --log-level=DEBUG \
-W error::UserWarning
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We actually would like to see the DeprecationWarning's as well (-W error without the specific UserWarning), but on Python 3.10 Numpy's testing import causes deprecation warnings due to the distutils future removal in Python 3.12. So, we would have to wait for Numpy's removal of distutils before we did that. Unless I'm missing a way to catch a specific string of deprecation warnings so we could ignore only that specific warning.

Copy link
Member

Choose a reason for hiding this comment

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

Sure, you can be more specific with warnings filters: https://docs.python.org/3/library/warnings.html#describing-warning-filters

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing that out. I tried a few things out and it was more complicated that I hoped.
See here for other discussion: https://bugs.python.org/issue43862

While the Python API warnings.filterwarnings(action, message="", ...) uses the message as a regular expression, -W and PYTHONWARNINGS require to match exactly the whole message.

I got one match, and then another import failed, so not sure how many options/messages we want to match on if we do. I couldn't figure out how to use the "module" piece either, I thought that could refer to errors raise from distutils directly, or even by putting numpy.testing in there for where the warnings were arising from, but neither of those worked for me...

In summary, I'd leave it with the UserWarning -> Error for now and deal with DeprecationWarnings after Numpy fixes their distutils imports.

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, you could change pytest.ini to ignore what you want, as it seems to support regex.

cat >> pytest.ini <<EOF
filterwarnings =
    error:message:DeprecationWarning
EOF

@tacaswell
Copy link
Member

I think some of the 3.10 + numpy-1.22.dev failures are fixed on numpy's default branch (at one point I was seeing issuse with distutils warnings, but they are gone for me locally).

I am in principle 👍🏻 👍🏻 on this idea!

@greglucas
Copy link
Contributor Author

Looks like the test suite is passing with the nightly pandas and numpy wheels now.

https://github.com/greglucas/matplotlib/runs/4290939395?check_suite_focus=true

run: |
xvfb-run -a python -mpytest -raR -n auto \
--maxfail=50 --timeout=300 --durations=25 --log-level=DEBUG \
-W error::UserWarning
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, you could change pytest.ini to ignore what you want, as it seems to support regex.

cat >> pytest.ini <<EOF
filterwarnings =
    error:message:DeprecationWarning
EOF

@greglucas
Copy link
Contributor Author

Some of the tests seem to be a little flaky when they are run: https://github.com/greglucas/matplotlib/actions/runs/1495511858
The new changes seem to be good, but test_errorbar is failing, even though it passed on a previous run with the same dependency versions installed...

@tacaswell tacaswell added this to the v3.6.0 milestone Nov 25, 2021
@greglucas
Copy link
Contributor Author

I pushed a local copy of this up to my fork: https://github.com/greglucas/matplotlib/runs/4621007693?check_suite_focus=true
and both 3.9 and 3.10 with latest numpy is failing. However, when I try to run this all locally (installing numpy from nightly wheels) the tests all pass for me. There must be something about the runner environment that makes a few of the tests flaky, but I haven't been able to figure it out yet looking through the logs.

@QuLogic
Copy link
Member

QuLogic commented Dec 23, 2021

The result images should be obtainable on the Summary page. It doesn't look like the failures are anywhere other than test images.

@greglucas
Copy link
Contributor Author

Yep, I did download them and couldn't find any obvious reason for the failures (they are all quite small). The largest one test_errorbar has an expanded y-limit, causing the number of ticks to be increased, but the data in the plot appears to be correct. My initial guess is that the failures are due to floating point discrepancies, causing a ylim to be expanded to the next decade, but I'm not sure where that would be coming from since I can't reproduce locally.

@greglucas
Copy link
Contributor Author

Putting some more notes down here:
This test suite does pass CI if I don't upgrade numpy to the nightly wheel: https://github.com/greglucas/matplotlib/actions/runs/1617303489 (I was worried I'd updated some other config to make a standard run fail). I also tried pip installing nightly numpy before everything else and after everything else (including matplotlib), to make sure it wasn't affecting the compilation of any dependencies and it failed in every case there as well.

I can't reproduce these failures on a local macosx or ubuntu machine either, so my debugging capability here is limited unfortunately.

@greglucas
Copy link
Contributor Author

Perhaps this is related to AVX512 instructions now being available in the wheel...
https://numpy.org/devdocs/release/1.22.0-notes.html#vectorize-umath-module-using-avx-512

On the 3.8-3.10 runners:

Supported SIMD extensions in this NumPy install:
    baseline = SSE,SSE2,SSE3
    found = SSSE3,SSE41,POPCNT,SSE42,AVX,F16C,FMA3,AVX2,AVX512F,AVX512CD,AVX512_SKX
    not found = AVX512_KNL,AVX512_KNM,AVX512_CLX,AVX512_CNL,AVX512_ICL

On the 3.7 runner (numpy 1.21):

Supported SIMD extensions in this NumPy install:
    baseline = SSE,SSE2,SSE3
    found = SSSE3,SSE41,POPCNT,SSE42,AVX,F16C,FMA3,AVX2
    not found = AVX512F,AVX512CD,AVX512_KNL,AVX512_KNM,AVX512_SKX,AVX512_CLX,AVX512_CNL,AVX512_ICL

My local Ubuntu is on a chip without AVX512 instructions, which could explain why I'm not able to reproduce this locally. If someone does have a system with a chip that supports AVX512 and could test locally that could verify this guess: python -c "import numpy; numpy.show_config()"
or if someone knows of a way to disable that instruction set from use within numpy at runtime rather than build time, I could push that up to the runner as well.

@dstansby
Copy link
Member

dstansby commented Jan 3, 2022

Perhaps this is related to AVX512 instructions now being available in the wheel... https://numpy.org/devdocs/release/1.22.0-notes.html#vectorize-umath-module-using-avx-512

I suspect this is correct - I'm seeing similar small floating point differences on another project that seems to be related to both numpy version and the specific machine CI is being run on.

@greglucas
Copy link
Contributor Author

I decided to rework this and incorporate it into the current tests.yml file, which seems better than trying to maintain two copies of the CI file. There are new floating-point issues that cropped up again in Normalize with longdouble. It must be a new numpy optimization in .min(), as specific item-access works, so I've updated the tests to account for these changes.

@greglucas greglucas force-pushed the future-deps branch 2 times, most recently from d0e5605 to b4d1b14 Compare February 2, 2022 15:33
Test future numpy and pandas versions with Matplotlib to see if
anything needs to be done in the future to address deprecations
or other pending changes.

Turns all warnings into errors, but filters out the distutils
deprecation and find_spec warnings.
@QuLogic QuLogic merged commit 0407b9f into matplotlib:main Feb 12, 2022
@greglucas greglucas deleted the future-deps branch February 12, 2022 15:15
Comment on lines 577 to +579
# This returns exactly 0.5 when longdouble is extended precision (80-bit),
# but only a value close to it when it is quadruple precision (128-bit).
assert 0 < norm(1 + 50 * eps) < 1
np.testing.assert_array_almost_equal_nulp(norm(1 + 50 * eps), 0.5)
Copy link
Member

Choose a reason for hiding this comment

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

This seems to be a bit too strict now, as it is failing on aarch64, ppc64le, and s390x, where the result is 0.50096339.

Also, on my 64-bit AMD system, which is apparently using np.float128 for np.longdouble (though I don't know if that means 80-bit internally), it seems to return 0.5 exactly, which seems the opposite of the comment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants