Conversation
…16.1, tested at the upper bounds and all tests pass
…d of year. Fix incorrect 3.9 spec in unit tests
dpanici
requested review from
a team,
YigitElma,
ddudt,
f0uriest,
rahulgaur104 and
unalmis
and removed request for
a team
Memory benchmark result| Test Name | %Δ | Master (MB) | PR (MB) | Δ (MB) | Time PR (s) | Time Master (s) |
| -------------------------------------- | ------------ | ------------------ | ------------------ | ------------ | ------------------ | ------------------ |
test_objective_jac_w7x | 1.76 % | 3.813e+03 | 3.880e+03 | 67.07 | 39.76 | 36.95 |
test_proximal_jac_w7x_with_eq_update | 2.59 % | 6.533e+03 | 6.703e+03 | 169.32 | 162.39 | 163.91 |
test_proximal_freeb_jac | 0.17 % | 1.323e+04 | 1.325e+04 | 22.92 | 85.71 | 85.93 |
test_proximal_freeb_jac_blocked | -0.70 % | 7.553e+03 | 7.500e+03 | -53.19 | 74.27 | 73.87 |
test_proximal_freeb_jac_batched | 0.54 % | 7.469e+03 | 7.510e+03 | 40.64 | 73.58 | 74.08 |
test_proximal_jac_ripple | -1.80 % | 3.535e+03 | 3.471e+03 | -63.73 | 66.67 | 66.98 |
test_proximal_jac_ripple_bounce1d | 2.59 % | 3.540e+03 | 3.632e+03 | 91.80 | 78.32 | 77.92 |
test_eq_solve | -1.28 % | 2.041e+03 | 2.015e+03 | -26.15 | 95.72 | 94.62 |For the memory plots, go to the summary of |
|
The diff from my pip list on stellar (where the failing test passes) and the one in the action that is failing is: Matplotlib does not explicitly say it supports Python 3.14 until pip apparently does not prevent 3.9.2 from installing on Python 3.14 despite there being issues, so how are we supposed to deal with this with simple requirements.txt files? |
For testing on CI, we install matplotlib 3.9.2 which doesn't support Python 3.14, I think that is the reason. I am not sure how to proceed about this though. |
|
yea I am not sure either. I tried to google how to deal with this and am just finding no helpful info, but did find an interesting article on version capping lol which argues the opposite of what I feel https://iscinumpy.dev/post/bound-version-constraints/ |
I feel like the chance of next JAX release breaking something is higher than 75%, so we should be good :) |
|
Update numpy and scipy to latest versions |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2079 +/- ##
==========================================
- Coverage 94.53% 94.49% -0.04%
==========================================
Files 102 102
Lines 28785 28787 +2
==========================================
- Hits 27211 27203 -8
- Misses 1574 1584 +10
🚀 New features to boost your workflow:
|
|
I think the infinite values in master data are due to the grid nodes on the magnetic axis. We compute equilibrium quantities on axis (to make sure the limits are correctly implemented) but some of these quantities are not well-defined and cause the issue. These are the problematic values: |
…reproduce locally
|
| # since eq grid has nodes on axis, some quantities are infinite | ||
| # so just use 1.0 as a scale for the tolerance | ||
| print(f"mean(|master data|) of {name} for {p} is not finite.") | ||
| mean = 1.0 |
There was a problem hiding this comment.
we could also instead do
this_data=np.atleast_1d(np.abs(master_data[p][name]))
finite_data = this_data[np.where(np.isfinite(this_data))]
mean = np.nanmean(finite_data)maybe nanmean not necessary here anymore, but this would keep the mean part. Maybe not needed though, if what you have works
There was a problem hiding this comment.
atol=1e-8 * mean + 1e-9 so I guess anything is fine
| # target force balance residuals with |F| <= 3e3 N | ||
| obj = ObjectiveFunction( | ||
| ForceBalance(normalize=False, normalize_target=False, bounds=(-1e3, 1e3), eq=eq) | ||
| ForceBalance(normalize=False, normalize_target=False, bounds=(-3e3, 3e3), eq=eq) |
There was a problem hiding this comment.
On my laptop, 1e3 passes, 2e3 fails, 3e3 and higher pass. I guess there are some edge cases for bounds
There was a problem hiding this comment.
For me locally, 1e3 2e3 and 3e3 all pass this test just fine. Seems something very sensitive to the local setup is going on?
There was a problem hiding this comment.
Did you use the new version of numpy and scipy? I tested this in a new environment with exactly the same as this PR
There was a problem hiding this comment.
oh no I did not use the new versions
There was a problem hiding this comment.
on stellar with Python 3.14 and scipy 1.17 and numpy 2.3.5 all three seem to pass for me. I did not check the rest of the environment for the PR though
There was a problem hiding this comment.
maybe numpy 2.4.2 has some changes?
YigitElma
left a comment
YigitElma
left a comment
There was a problem hiding this comment.
Someone else should also review
|
Add python-version file to use 3.13, setup.py will still have 3.10 until we discontinue support for that at its end of life @dpanici |
dpanici
left a comment
dpanici
left a comment
There was a problem hiding this comment.
I can't approve as I made the main changes but I'd approve, this looks good to me and hopefully fixes dependabot now
3 participants
Resolves #2070
Nothing major changed. Main things are
1.17.0, matplotlib to3.10.8for numpy to2.4.2as those are the newest versions, which also have Python 3.14 support (though note that pip never complained when I installed with an upper bound for numpy which did not support 3.14, which was annoying...)2.3.5so that seems to be the cap based on our other dependenciesdispforl-bfgs-bwhich was unused anyways, so had to add a check to pop that from options in that caseHowever
matplotlib<3.10.5is broken with Python 3.14 due to some lowlevel changes in cPython [MNT]: Python 3.14: multiple RecursionErrors when running tests matplotlib/matplotlib#29185So a possible way forward I guess is to just