Closed BradyPlanden closed 1 year ago
I think the error is originating from the fact that the dev
environment tries to install scikits.odes
but never installs SUNDIALS
(in the tox
env). tox -e pybamm-requires
on the other hand does install SUNDIALS
in the tox
env -
commands =
python {toxinidir}/scripts/install_KLU_Sundials.py
- git clone https://github.com/pybind/pybind11.git {toxinidir}/pybind11
The tox
file is a bit messy. For instance, look how jax
and jaxlib
are never installed during unit and integration tests -
commands =
tests-!windows-!mac: sh -c "pybamm_install_jax" # install jax, jaxlib for ubuntu
tests: python run-tests.py --all
unit: python run-tests.py --unit
integration: python run-tests.py --integration
allowing the testing suite to skip the test for JaxSolver
. But, JaxSolver
is tested when the coverage report is generated on Python 3.9
-
[testenv:coverage]
deps =
coverage
scikits.odes
commands =
!windows-!mac: sh -c "pybamm_install_jax"
coverage run run-tests.py --nosub
# Some tests make use of multiple processes through
# multiprocessing. Coverage data is then generated for each
# process separately and data must then be combined into one
# single coverage data file.
coverage combine
coverage xml
I am hoping these discrepancies will be solved under GSoC this year :)
I think executing tox -e dev
should give a similar error on Ubuntu. I might be wrong though.
To confirm, this is reproducible on both mac and linux when installing via the brew method. At the moment a quick fix is to specify the following environment variables prior to running tox -e dev
:
export CMAKE_PREFIX_PATH="$(brew --prefix)"
export SUNDIALS_INST="$(brew --prefix sundials)"
I'm just looking into a cleaner solution...
Based on #1296 perhaps a way to solve this would be to update the documentation for installing from source, so that the instructions say that:
brew install sundials
in developer installations since it does not link the cmake
args that @jsbrittain mentions — on the other hand, this is something that scripts/install_klu_sundials.py
does, so recommend that users use it insteadand
tox
, since most developers and contributors are using tox
to run the tests. The pybamm-requires
env, i.e., the tox -e pybamm-requires
command, installs SUNDIALS
with the above script correctly (with install_klu_sundials
) and not with brew
Yes, I think directing them to pybamm-requires
seems the most straightforward and provides a clear recommended developer procedure, and has already been noted to solve the original Issue as raised by @BradyPlanden .
Just note that the Mac install does still rely on brew
for openblas
, etc., and when testing this approach (admittedly in a linux container, i.e. not the recommended approach), pybamm-requires
then had problems finding blas
until directed towards the brew install. Testing on Mac with brew
installed to the default location seemed to work fine though.
I'll update and simplify the install docs in accordance with @agriyakhetarpal suggestion above, specifically point 2.
PyBaMM Version
Source
Python Version
3.10 & 3.11
Describe the bug
Installing from source results in the following sundials header error via the
brew install sundials
method for M-series hardware. Using,python3 -m tox -e pybamm-requires
fixes the issue. It appears to be a linking issue, as thebrew install sundials
doesn't appear to install libraries in~/.local
. Seems close to #1296Steps to Reproduce
Follow MacOS installation via https://pybamm.readthedocs.io/en/stable/source/user_guide/installation/install-from-source.html#install-from-source-developer-install
Relevant log output