-
Notifications
You must be signed in to change notification settings - Fork 118
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
Support numpy>=2
and remove numpy<2
pin
#3040
Conversation
This comment has been minimized.
This comment has been minimized.
✔️ 8904530 -> Azure artifacts URL |
numpy>=2
and remove numpy<2
pin
✔️ 5ba690e -> Azure artifacts URL |
4d2bfd8
to
fb0b38c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3040 +/- ##
==========================================
- Coverage 67.27% 67.26% -0.01%
==========================================
Files 571 571
Lines 104882 104887 +5
==========================================
- Hits 70555 70550 -5
- Misses 34327 34337 +10 ☔ View full report in Codecov by Sentry. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
ef44b28
to
af837ba
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✔️ 24e493b -> Azure artifacts URL |
✔️ d30ad08 -> Azure artifacts URL |
✔️ 16d874e -> Azure artifacts URL |
✔️ 95b461c -> Azure artifacts URL |
✔️ 7b9e1be -> Azure artifacts URL |
Wheels are now built with the latest Numpy and tested with the oldest Numpy that supports the corresponding Python version. A few notes:
|
The "might take" is measurable. How long do they take? How long is that compared to building NEURON? |
@kbvw > Python 3.8 kept failing over many runs of the CI. Can you point me to a couple of those? I'm interested to see how it failed. |
This comment has been minimized.
This comment has been minimized.
They're a bit hard to find back because I force-pushed to the branch so often while debugging, but here is one example. This particular time, it was built with Numpy
This was one in a range of runs where every time, it was built with To be clear about what changed from before: we used to build the wheels with the oldest supported Numpy, and then test with the latest. As of Numpy 2, as you can read here, wheels built with |
@kbvw > 'from neuron import rxd' failed numpy.ndarray size changed, may indicate binary incompatibility. Expected 96 from C header, got 88 from PyObject Neat, thanks for the answer. That's what I was wondering. I remember running into similar problems in going from 1.19 to 1.20, since the size of ndarray changed. I'm wondering if the build system is pickup various numpy headers instead of the ones that are specified?
That's very interesting. I wonder how they pulled that trick off, as even in the 1.xx days they couldn't guarantee backwards compat. The docs suggest it's for all the 1.xx series, but I wonder how they do that. Not important for this PR, just another thing I'd like to know. |
✔️ b624267 -> Azure artifacts URL |
From the latest CI run, over the range of Linux and Mac Python wheels: 13m43s to 20m17s for the build script So overal about a 9-11% increase. Quickly testing with latest and oldest NumPy seems useful (especially instead of only oldest), but not sure if everything needs to be tested twice: MPI, nrnivmodl, etc. Maybe I'll re-arrange the script and just do |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✔️ a665da1 -> Azure artifacts URL |
This comment has been minimized.
This comment has been minimized.
When running only |
Quality Gate passedIssues Measures |
✔️ 9fb93ec -> Azure artifacts URL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work.
No description provided.