BLD: NumPy 2 compat for wheel builds #4620

tylerjereddy · 2024-06-18T11:38:39Z

Related to CI fails when numpy 2.0.0 is installed #4619
We should build against NumPy 2.0.0 so that we're backwards compatible to NumPy 1.x while still supporting 2.x series. This is simpler to maintain as well. In theory, should support as far back as NumPy 1.19 if we needed it.

PR Checklist

Tests?
Docs?
CHANGELOG updated?
Issue raised/referenced?

Developers certificate of origin

I certify that this contribution is covered by the LGPLv2.1+ license as defined in our LICENSE and adheres to the Developer Certificate of Origin.

📚 Documentation preview 📚: https://mdanalysis--4620.org.readthedocs.build/en/4620/

* Fixes MDAnalysis#4619 * We should build against NumPy `2.0.0` so that we're backwards compatible to NumPy `1.x` while still supporting 2.x series. This is simpler to maintain as well. In theory, should support as far back as NumPy `1.19` if we needed it.

github-actions · 2024-06-18T11:40:59Z

Linter Bot Results:

Hi @tylerjereddy! Thanks for making this PR. We linted your code and found the following:

Some issues were found with the formatting of your code.

Code Location	Outcome
main package	⚠️ Possible failure
testsuite	⚠️ Possible failure

Please have a look at the darker-main-code and darker-test-code steps here for more details: https://github.com/MDAnalysis/mdanalysis/actions/runs/9625990228/job/26551462207

Please note: The black linter is purely informational, you can safely ignore these outcomes if there are no flake8 failures!

tylerjereddy · 2024-06-18T11:49:08Z

some of our Windows deps may also not have NumPy 2 compatible releases yet, need to check on that

codecov · 2024-06-18T12:05:32Z

Codecov Report

Attention: Patch coverage is 42.85714% with 8 lines in your changes missing coverage. Please review.

Project coverage is 93.57%. Comparing base (d2d9d27) to head (7fd4925).
Report is 24 commits behind head on develop.

Files with missing lines	Patch %	Lines
package/MDAnalysis/converters/ParmEd.py	33.33%	5 Missing and 1 partial ⚠️
package/MDAnalysis/converters/RDKit.py	60.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4620      +/-   ##
===========================================
- Coverage    93.63%   93.57%   -0.06%     
===========================================
  Files          171      183      +12     
  Lines        21225    22304    +1079     
  Branches      3930     3933       +3     
===========================================
+ Hits         19873    20872     +999     
- Misses         894      972      +78     
- Partials       458      460       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

IAlibay

Thanks @tylerjereddy - just the one question.

I have a suspicion some of our gh actions CI won't be picking this at all since we do a lot of isolation disabling here and there, but if the azure pipelines CI works then it should be good.

IAlibay · 2024-06-18T14:46:05Z

package/pyproject.toml

-    # NumPy version. In that case we just let it be a bare NumPy install
-    "numpy<2.0; python_version>='3.13'",
+    # numpy requirement for wheel builds for distribution on PyPI - building
+    # against 2.x yields wheels that are also compatible with numpy 1.x at


Is the range of compatible 1.x numpy versions with 2.0 published somewhere? I.e. checking if we need to bump the minimum NumPy version anywhere.

This document isn't too bad: https://numpy.org/doc/stable/dev/depending_on_numpy.html#for-downstream-package-authors. I think we may be "ok" on that front.

You can deduce the stability back to 1.25.x and then to 1.19.x based on that document. It looks like we have Python 3.10 and NumPy 1.23.2 as our runtime lower bounds, which is pretty close to where SciPy is (3.10 and 1.23.5 respectively) at the moment.

Let me do a quick check of the current CI failures to see if I can make a concrete suggestion for those as well.

* Related to MDAnalysis/mdanalysis#4620. * See discussion above for details, but in short we should build our wheels against NumPy `2.0.0` so that we have binaries that are runtime-compatible with both NumPy 1.x and 2.x series. * Even if "we" don't do a release soon, I believe this may be helpful since we could then point the MDAnalysis CI at this github repo for `pytng` "no binary" build from source with NumPy 2.x, rather than disabling `pytng` testing over there.

tylerjereddy · 2024-06-18T17:09:10Z

For pytng errors, I think the cross-listed PR is roughly what we'd want to do there. Even if there's no bandwidth for a release for pytng (let alone MDA) for a bit, we could perhaps point to the GitHub master branch of pytng for installs in our CI here (i.e., point pip at the GitHub repo) to keep that testing active rather than disabling pytng testing here.

There may be a few other test failures here to check, but the pytng ones stood out first.

I've adjusted the original comment to not close the matching issue, since this may be a step in the right direction, but we may need to check other deps/things a bit still.

* Related to MDAnalysis/mdanalysis#4620. * See discussion above for details, but in short we should build our wheels against NumPy `2.0.0` so that we have binaries that are runtime-compatible with both NumPy 1.x and 2.x series. * Even if "we" don't do a release soon, I believe this may be helpful since we could then point the MDAnalysis CI at this github repo for `pytng` "no binary" build from source with NumPy 2.x, rather than disabling `pytng` testing over there.

tylerjereddy · 2024-06-19T15:32:27Z

I restarted the failed Azure jobs to see if the new pytng release helps the situation a bit.

tylerjereddy · 2024-06-19T16:24:12Z

Looking at the logs, in terms of our dependencies, I'd say parmed seems to have moderate issues with NumPy 2 (they need to adjust their array copy semantics a bit like we did). RDKit seems to have a more serious problem, resulting in hard crashes against NumPy 2. Both projects probably need a new release to support.

IAlibay · 2024-06-19T17:19:38Z

@tylerjereddy sorry I'm going to word this terribly - these are optional dependencies, is there a path you can see here where we stick to 1.x if you need these optional dependencies and 2+ if you don't? I guess in we'd effectively need two packages with different pins?

My view is that this doesn't seem like it would work, but I've put all of five minutes of thought into it.

* Add NumPy 2-related shims aimed at avoiding control flow that hits `RDKit` or `parmed`, until they have releases supporting NumPy 2. This mostly means skipping a bunch of tests that use those packages for now.

pep8speaks · 2024-06-19T22:08:26Z

Hello @tylerjereddy! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file testsuite/MDAnalysisTests/util.py:

Line 42:1: E402 module level import not at top of file
Line 43:1: E402 module level import not at top of file

Comment last updated at 2024-06-22 13:59:40 UTC

tylerjereddy · 2024-06-19T23:12:59Z

@IAlibay I think the best I can suggest is to skip the related tests/imports for parmed and rdkit under NumPy 2 until they have compatible releases available.

I pushed in a sample commit that seems to allow the testsuite to pass locally with NumPy 2. Looking at the results in CI here, the only test failure is test_creating_multiple_universe_without_offset, which I believe is a known issue/flake.

That doesn't mean the MDA team will like this, but I pushed it in so you can take a look. Maybe not too painful?

IAlibay · 2024-06-22T13:29:01Z

@IAlibay I think the best I can suggest is to skip the related tests/imports for parmed and rdkit under NumPy 2 until they have compatible releases available.

I pushed in a sample commit that seems to allow the testsuite to pass locally with NumPy 2. Looking at the results in CI here, the only test failure is test_creating_multiple_universe_without_offset, which I believe is a known issue/flake.

That doesn't mean the MDA team will like this, but I pushed it in so you can take a look. Maybe not too painful?

O wow thanks a bunch for digging into this @tylerjereddy !
I don't mind this solution, it's not "clean", but it's the "cleanest" I think we can do here - rdkit will update eventually, parmed might not and there's nothing we can do about that.

IAlibay

Thanks so much @tylerjereddy ! I hope you don't mind I took the liberty of pushing a few commits directly to make sure we can get this merged in asap.

IAlibay · 2024-06-22T13:39:32Z

package/MDAnalysis/converters/ParmEd.py

-            import parmed as pmd
+            # TODO: remove this guard when parmed has a release
+            # that supports NumPy 2
+            if NumpyVersion(np.__version__) < "2.0.0":


I just added this extra guard here since we were doing the same for RDKit.

IAlibay · 2024-06-22T14:32:14Z

Thanks @tylerjereddy - going with the merge!

tylerjereddy added the maintainability label Jun 18, 2024

IAlibay reviewed Jun 18, 2024

View reviewed changes

tylerjereddy mentioned this pull request Jun 18, 2024

BLD: build wheels against NumPy 2.0.0 MDAnalysis/pytng#111

Merged

MAINT, TST: PR 4620 revisions

9e9d25f

* Add NumPy 2-related shims aimed at avoiding control flow that hits `RDKit` or `parmed`, until they have releases supporting NumPy 2. This mostly means skipping a bunch of tests that use those packages for now.

github-actions bot added the Component-Converters label Jun 19, 2024

IAlibay mentioned this pull request Jun 22, 2024

Installing MDAnalysis from pip deps triggers numpy error MDAnalysis/cookiecutter-mdakit#129

Closed

Update ParmEd.py

bb0de71

github-actions bot removed the Component-Converters label Jun 22, 2024

Update CHANGELOG

3cb32c2

IAlibay approved these changes Jun 22, 2024

View reviewed changes

This was referenced Jun 22, 2024

MDAnalysis develop incompatible with numpy 2.0 MDAnalysis/install-mdanalysis#9

Closed

Unpin rdkit imports once rdkit supports numpy 2.0 #4624

Closed

Unpin parmed imports once parmed supports numpy 2.0 #4625

Open

Update ParmEd.py

7fd4925

IAlibay merged commit cfa4438 into MDAnalysis:develop Jun 22, 2024
21 of 23 checks passed

tylerjereddy deleted the treddy_issue_4619 branch June 22, 2024 20:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BLD: NumPy 2 compat for wheel builds #4620

BLD: NumPy 2 compat for wheel builds #4620

tylerjereddy commented Jun 18, 2024 •

edited by IAlibay

Loading

github-actions bot commented Jun 18, 2024 •

edited

Loading

tylerjereddy commented Jun 18, 2024

codecov bot commented Jun 18, 2024 •

edited

Loading

IAlibay left a comment •

edited

Loading

IAlibay Jun 18, 2024

tylerjereddy Jun 18, 2024

tylerjereddy Jun 18, 2024

tylerjereddy commented Jun 18, 2024 •

edited

Loading

tylerjereddy commented Jun 19, 2024

tylerjereddy commented Jun 19, 2024

IAlibay commented Jun 19, 2024

pep8speaks commented Jun 19, 2024 •

edited

Loading

tylerjereddy commented Jun 19, 2024

IAlibay commented Jun 22, 2024

IAlibay left a comment

IAlibay Jun 22, 2024

IAlibay commented Jun 22, 2024

BLD: NumPy 2 compat for wheel builds #4620

BLD: NumPy 2 compat for wheel builds #4620

Conversation

tylerjereddy commented Jun 18, 2024 • edited by IAlibay Loading

PR Checklist

Developers certificate of origin

github-actions bot commented Jun 18, 2024 • edited Loading

Linter Bot Results:

tylerjereddy commented Jun 18, 2024

codecov bot commented Jun 18, 2024 • edited Loading

Codecov Report

IAlibay left a comment • edited Loading

Choose a reason for hiding this comment

IAlibay Jun 18, 2024

Choose a reason for hiding this comment

tylerjereddy Jun 18, 2024

Choose a reason for hiding this comment

tylerjereddy Jun 18, 2024

Choose a reason for hiding this comment

tylerjereddy commented Jun 18, 2024 • edited Loading

tylerjereddy commented Jun 19, 2024

tylerjereddy commented Jun 19, 2024

IAlibay commented Jun 19, 2024

pep8speaks commented Jun 19, 2024 • edited Loading

Comment last updated at 2024-06-22 13:59:40 UTC

tylerjereddy commented Jun 19, 2024

IAlibay commented Jun 22, 2024

IAlibay left a comment

Choose a reason for hiding this comment

IAlibay Jun 22, 2024

Choose a reason for hiding this comment

IAlibay commented Jun 22, 2024

tylerjereddy commented Jun 18, 2024 •

edited by IAlibay

Loading

github-actions bot commented Jun 18, 2024 •

edited

Loading

codecov bot commented Jun 18, 2024 •

edited

Loading

IAlibay left a comment •

edited

Loading

tylerjereddy commented Jun 18, 2024 •

edited

Loading

pep8speaks commented Jun 19, 2024 •

edited

Loading