-
-
Notifications
You must be signed in to change notification settings - Fork 529
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
Updated scipy and jax versions to fix linalg.tri
deprecation
#4103
Conversation
@brosaplanella It looks like the remaining failures are about precision |
Ok, I found the issue. Basically they have changed the solver for the interpolant and now it is an iterative one (see Scipy docs). I have now defined the interpolant to use a direct solver (as before) so accuracy should remain. If needed, in the future, we can allow users to specify the solver when creating the interpolant. EDIT: just realised that this new feature is only supported from 1.13 so it would break with older Scipy versions. @valentinsulzer, how should we deal with this? Pin Scipy to 1.13 or is there a way to support both? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #4103 +/- ##
===========================================
- Coverage 99.56% 99.54% -0.02%
===========================================
Files 287 287
Lines 21744 21744
===========================================
- Hits 21649 21646 -3
- Misses 95 98 +3 ☔ View full report in Codecov by Sentry. |
@brosaplanella If the only issue was precision before switching to the direct solver, then I would guess that the iterative solver would be fine. If they made that the default, then there is probably a reason for it |
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
…m/PyBaMM into issue-3959-linalg-deprecation
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.
Assuming tests pass, then I am happy with this
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.
Thanks for taking the time to do this, @brosaplanella! The changes here were a bit easier than I expected they would be, just one comment on the version bump
…m/PyBaMM into issue-3959-linalg-deprecation
Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com>
… macos failing test)
…m-team#4103) * pybamm-team#3959 updated scipy and jax versions to fix deprecation error * pybamm-team#3959 fix issue with vstack array dimensions * style: pre-commit fixes * pybamm-team#3959 use direct solver for interpolant * Update pyproject.toml Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> * pybamm-team#3959 use jax and jaxlib 0.4.27 * pybamm-team#3959 revert to iterative solver for interpolator and relax test tolerances * style: pre-commit fixes * ruff * pybamm-team#3959 Eric's comments * pybamm-team#3959 reduce tolerances to fix macos-14 unit tests * Update pyproject.toml Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> * pybamm-team#3959 relax some more tolerances * pybamm-team#3959 reduce solve time in test to avoid overdischarge (aiming to fix macos failing test) * pybamm-team#3959 extend solving time to reach end of discharge --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Agriya Khetarpal <74401230+agriyakhetarpal@users.noreply.github.com> Co-authored-by: Eric G. Kratz <kratman@users.noreply.github.com>
Description
Updated jax & jaxlib to most recent version and scipy to >= 1.11.0 (the one that introduced the deprecation warning).
Fixes #3959
Type of change
Please add a line in the relevant section of CHANGELOG.md to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.
Key checklist:
$ pre-commit run
(or$ nox -s pre-commit
) (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)$ python run-tests.py --all
(or$ nox -s tests
)$ python run-tests.py --doctest
(or$ nox -s doctests
)You can run integration tests, unit tests, and doctests together at once, using
$ python run-tests.py --quick
(or$ nox -s quick
).Further checks: