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

Allow pybamm-requires to return early if an existing SUNDIALS installation is found #3418

Closed
agriyakhetarpal opened this issue Oct 6, 2023 · 3 comments · Fixed by #3719
Closed
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve

Comments

@agriyakhetarpal
Copy link
Member

agriyakhetarpal commented Oct 6, 2023

The pybamm-requires session, activated by the nox -s pybamm-requires command, can be sped up by detecting the build-time components in the $HOME directory. Note: this would be similar to what pybamm_install_odes already does; the script can find an installation of SUNDIALS and move to the next steps, i.e., installing scikits.odes.

However, in cases of a corrupt installation of SuiteSparse and SUNDIALS, we can force the re-installation by nox's posargs feature: providing the -f or the --force command-line flag when invoking the session will re-compile them. We already do this for the pybind11 directory in the session by not cloning the repository if it is found.

This way, users can skip the re-installation of the build-time components if already present but still factor in the cases where it is necessary for them to reinstall for debugging purposes. Another way this will help will be to skip the session in CI if a cache-hit occurs (we don't need to bother about validating the caches because GitHub Actions runners would already do that for us)

@agriyakhetarpal agriyakhetarpal added difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve hacktoberfest labels Oct 6, 2023
@Saswatsusmoy
Copy link

import os
from pathlib import Path

BUILD_TIME_COMPONENTS = ["suitesparse", "sundials"]

def should_skip_component(component):
return Path(f"{os.environ['HOME']}/{component}").exists()

def install_build_time_components():
for component in BUILD_TIME_COMPONENTS:
if should_skip_component(component):
print(f"{component} already installed, skipping...")
continue

    print(f"Installing {component}")

def main():
force = "--force" in os.environ.get("ARGS", "")

if force or any(not should_skip_component(c) for c in BUILD_TIME_COMPONENTS):
    install_build_time_components()

if name == "main":
main()

Will something like this work?

Define list of build time components to check
Check if component already exists in home directory
If component exists, skip installing it
Allow force reinstall with --force argument
Main function checks all components and conditionally installs them if: force argument provided, any component does not already exist
This allows skipping already installed components while still allowing force reinstall if needed.

@agriyakhetarpal
Copy link
Member Author

Yes; something like this should work fine, though the details of its implementation would vary slightly since this is to be added in the pybamm-requires session inside noxfile.py and therefore has to use commonly-used nox API syntax (https://nox.thea.codes/en/stable/)

The SUNDIALS and SuiteSparse installation locations can be determined by reading through scripts/install_KLU_Sundials.py and a method to detect an existing SUNDIALS installation can be found in pybamm/install_odes.py, which checks if relevant dynamic libraries or shared object files exist in the installation folder. The process to detect SuiteSparse should be similar.

@Saswatsusmoy
Copy link

Saswatsusmoy commented Oct 17, 2023

I've tried to solve the issue, would appreciate feedback

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: easy A good issue for someone new. Can be done in a few hours priority: low No existing plans to resolve
Projects
None yet
2 participants