Skip to content
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

Extend pybamm_install_odes to include support for macOS systems #3404

Closed
agriyakhetarpal opened this issue Oct 3, 2023 · 5 comments · Fixed by #3417
Closed

Extend pybamm_install_odes to include support for macOS systems #3404

agriyakhetarpal opened this issue Oct 3, 2023 · 5 comments · Fixed by #3417
Assignees
Labels
difficulty: medium Will take a few days priority: low No existing plans to resolve

Comments

@agriyakhetarpal
Copy link
Member

The pybamm/install_odes.py script, i.e., the pybamm_install_odes entry point currently installs the build-time requirements (SUNDIALS), sets relevant environment variables, and scikits.odes for use by the optional solver. Considering that users can also use scikits.odes on macOS by pointing to a SUNDIALS installation using the SUNDIALS_INST environment variable, we can add suitable modifications to this script to make it work on all UNIX-based systems (macOS and Linux).

Some reference can be used from the install_KLU_Sundials.py which currently works for both macOS and Linux. However, a difference to be noted is that install_odes.py does not compile the KLU module, whereas the aforementioned script does.

This will help simplify the installation of scikits.odes and enable doing so without the usage of a nox session, and can then be tested in CI.

@agriyakhetarpal agriyakhetarpal added difficulty: medium Will take a few days hacktoberfest priority: low No existing plans to resolve labels Oct 3, 2023
@arjxn-py
Copy link
Member

arjxn-py commented Oct 4, 2023

Thanks for opening this @agriyakhetarpal.
Can I have a look at the logs when you execute pybamm/install_odes.py on your mac?

@agriyakhetarpal
Copy link
Member Author

Installation logs

$ pybamm_install_odes       
/Users/agriyakhetarpal/Desktop/PyBaMM/pybamm/scikits_odes_setup.log
Adding /Users/agriyakhetarpal/.local/lib to LD_LIBRARY_PATH in /Users/agriyakhetarpal/Desktop/PyBaMM/venv/bin/activate
Collecting scikits.odes
  Downloading scikits.odes-2.7.0.tar.gz (931 kB)
     ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 931.8/931.8 kB 6.0 MB/s eta 0:00:00
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Requirement already satisfied: scipy in ./venv/lib/python3.11/site-packages (from scikits.odes) (1.11.3)
Requirement already satisfied: numpy<1.28.0,>=1.21.6 in ./venv/lib/python3.11/site-packages (from scipy->scikits.odes) (1.26.0)
Building wheels for collected packages: scikits.odes
  Building wheel for scikits.odes (pyproject.toml) ... done
  Created wheel for scikits.odes: filename=scikits.odes-2.7.0-cp311-cp311-macosx_13_0_arm64.whl size=885590 sha256=9f954addb89481d0c6c169143aaa0267b816dc008ec8f901d1890283ae6c9373
  Stored in directory: /Users/agriyakhetarpal/Library/Caches/pip/wheels/41/7b/df/a45628ef0daaa6d2c60461a092198e27eaa4f72d995ed524a4
Successfully built scikits.odes
Installing collected packages: scikits.odes
Successfully installed scikits.odes-2.7.0

From scikits_odes_setup.log

2023-10-04 22:48:38,065 - scikits.odes setup - INFO - Starting scikits.odes setup
2023-10-04 22:48:38,065 - scikits.odes setup - INFO - Looking for sundials at /Users/agriyakhetarpal/.local
2023-10-04 22:48:38,065 - scikits.odes setup - INFO - Found sundials at /Users/agriyakhetarpal/.local

Looks like the output was truncated because I already have a SUNDIALS installation from nox -s pybamm-requires. So I guess it should be possible (and easier than I expected) to modify it, we just need to build without KLU (i.e., "-DENABLE_KLU=OFF" in the cmake_args, which I suspect is the default behaviour, so it can be omitted as well. Another thing to handle would be finding OpenMP on macOS similar to install_KLU_Sundials.py. Do you want to take this?

@agriyakhetarpal
Copy link
Member Author

It should also be fine to build with KLU and reuse an existing SUNDIALS installation like it did for my case, because scikits.odes does not provide an interface for it at all

@arjxn-py
Copy link
Member

arjxn-py commented Oct 5, 2023

Do you want to take this?

Sure, happy to pick it up.

@agriyakhetarpal
Copy link
Member Author

agriyakhetarpal commented Oct 5, 2023

Idea: enable KLU and add logic for finding OpenMP in order for users and developers to reuse the SUNDIALS files, though this would work only for source installations and not for pip installations (which come bundled with the dynamic libraries already). Another thing that would be helpful would be to let users exit early from pybamm-requires by detecting SUNDIALS at runtime, this would help both locally and in CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium Will take a few days priority: low No existing plans to resolve
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants