pybamm-team / PyBaMM

Fast and flexible physics-based battery models in Python
https://www.pybamm.org/
BSD 3-Clause "New" or "Revised" License
876 stars 492 forks source link

#4183 remove autograd #4196

Closed valentinsulzer closed 2 weeks ago

valentinsulzer commented 2 weeks ago

Description

Removes autograd dependency and thus enables update to numpy 2.0. The "differentiate numpy function directly" functionality, which relied on autograd, is deprecated, but I am not aware of any cases where it was being used, since we switched to implementing our own functions and specifying their derivatives.

Fixes #4183

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:

You can run integration tests, unit tests, and doctests together at once, using $ python run-tests.py --quick (or $ nox -s quick).

Further checks:

agriyakhetarpal commented 2 weeks ago

Thanks! Could we unpin NumPy with this? It was recently pinned in #4186

valentinsulzer commented 2 weeks ago

Converting to draft as the interpolant differentation issue should be fixable

valentinsulzer commented 2 weeks ago

Not obvious why test_parameter_values.test_repr is failing, and I'm not able to reproduce it locally.

Edit - looks like it was an issue caused by numpy 2.0, so I have kept numpy pinned to <2.0

codecov[bot] commented 2 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.54%. Comparing base (088ca38) to head (d38117b). Report is 3 commits behind head on develop.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #4196 +/- ## =========================================== - Coverage 99.58% 99.54% -0.05% =========================================== Files 288 288 Lines 21799 21857 +58 =========================================== + Hits 21708 21757 +49 - Misses 91 100 +9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

agriyakhetarpal commented 2 weeks ago

The problem with NumPy v2 is with this property that works on self._dict_items for pybamm.Scalar that I remember touching the code for before:

@value.setter
def value(self, value):
    self._value = (
        np.float64(value.item())
        if isinstance(value, np.ndarray)
        else np.float64(value)
    )

which with NumPy v2 returns np.float64(1.602176634e-19) rather than just 1.602176634e-19, i.e., the repr changed for NumPy v2. Could we unpin it and either ignore this error, or just extract the value without conversion to np.float64, or fix the repr in the test?

valentinsulzer commented 2 weeks ago

I also got this error with numpy 2.0 when trying to run locally:

/Users/vsulzer/Code/PyBaMM/pybamm/expression_tree/symbol.py:10: UserWarning: A NumPy version >=1.22.4 and <1.29.0 is required for this version of SciPy (detected version 2.0.0)
  from scipy.sparse import csr_matrix, issparse

A module that was compiled using NumPy 1.x cannot be run in
NumPy 2.0.0 as it may crash. To support both 1.x and 2.x
versions of NumPy, modules must be compiled with NumPy 2.0.
Some module may need to rebuild instead e.g. with 'pybind11>=2.12'.

If you are a user of the module, the easiest solution will be to
downgrade to 'numpy<2' or try to upgrade the affected module.
We expect that some modules will need time to support NumPy 2.

Traceback (most recent call last):  File "/Users/vsulzer/Code/PyBaMM/examples/scripts/compare_lithium_ion.py", line 4, in <module>
    import pybamm
  File "/Users/vsulzer/Code/PyBaMM/pybamm/__init__.py", line 26, in <module>
    from .expression_tree.symbol import *
  File "/Users/vsulzer/Code/PyBaMM/pybamm/expression_tree/symbol.py", line 10, in <module>
    from scipy.sparse import csr_matrix, issparse
  File "/Users/vsulzer/miniforge3/envs/pybamm/lib/python3.11/site-packages/scipy/sparse/__init__.py", line 295, in <module>
    from ._csr import *
  File "/Users/vsulzer/miniforge3/envs/pybamm/lib/python3.11/site-packages/scipy/sparse/_csr.py", line 11, in <module>
    from ._sparsetools import (csr_tocsc, csr_tobsr, csr_count_blocks,
AttributeError: _ARRAY_API not found
Traceback (most recent call last):
  File "/Users/vsulzer/Code/PyBaMM/examples/scripts/compare_lithium_ion.py", line 4, in <module>
    import pybamm
  File "/Users/vsulzer/Code/PyBaMM/pybamm/__init__.py", line 26, in <module>
    from .expression_tree.symbol import *
  File "/Users/vsulzer/Code/PyBaMM/pybamm/expression_tree/symbol.py", line 10, in <module>
    from scipy.sparse import csr_matrix, issparse
  File "/Users/vsulzer/miniforge3/envs/pybamm/lib/python3.11/site-packages/scipy/sparse/__init__.py", line 295, in <module>
    from ._csr import *
  File "/Users/vsulzer/miniforge3/envs/pybamm/lib/python3.11/site-packages/scipy/sparse/_csr.py", line 11, in <module>
    from ._sparsetools import (csr_tocsc, csr_tobsr, csr_count_blocks,
ImportError: numpy.core.multiarray failed to import

So I think we should leave it pinned and figure out unpinning it properly after the release