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

2447 cache Linux (apt) dependencies in CI #2923

Merged

Conversation

agriyakhetarpal
Copy link
Member

@agriyakhetarpal agriyakhetarpal commented May 3, 2023

Description

Caches dependencies for faster installation and setup of PyBaMM's dependencies via the apt package manager on Linux

Fixes #2447
Fixes #2632

Tasks

  • Cache pip dependencies (setup-python and actions/cache do not provide any benefits since most of our dependencies are installed within Tox environments)
  • Setup a cache for apt dependencies (partially done)
  • Investigate caching for brew dependencies (unhelpful)

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.

  • Optimization (back-end change that speeds up the code)

Key checklist:

  • No style issues: $ pre-commit run (see CONTRIBUTING.md for how to set this up to run automatically when committing locally, in just two lines of code)
  • All tests pass: $ python run-tests.py --all
  • The documentation builds: $ python run-tests.py --doctest

You can run unit and doctests together at once, using $ python run-tests.py --quick.

Further checks:

  • Code is commented, particularly in hard-to-understand areas

@codecov
Copy link

codecov bot commented May 3, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (a1e64c7) 99.71% compared to head (893bb6a) 99.71%.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #2923   +/-   ##
========================================
  Coverage    99.71%   99.71%           
========================================
  Files          245      245           
  Lines        18705    18705           
========================================
  Hits         18651    18651           
  Misses          54       54           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@agriyakhetarpal
Copy link
Member Author

We could also look into caching entire pip installations as well and their related directories, but I am not doing this for now may cause issues when updating Python versions or with dependencies that need post-install configuration steps (relevant discussion on actions/setup-python#330)

@valentinsulzer
Copy link
Member

How do we know if this is working?

@agriyakhetarpal
Copy link
Member Author

agriyakhetarpal commented May 4, 2023

We will need to compare the runtime for the step Setup Python ${{ matrix.python-version }} in the jobs with that of previous runs, and similarly for other package managers. It did not find any cache in most of the runs right now but I guess it should do so after a few iterations

Edit: a cache of 224 MB was generated which seems to be in line with the usual size of pip-installed packages. Though it did not speed up the workflow, I guess we are probably better off with it than without it?

@agriyakhetarpal
Copy link
Member Author

agriyakhetarpal commented May 4, 2023

The awalsh/cache-apt-pkgs-action took just short of 8 minutes to install dependencies, and 5 minutes to cache them – with a cache size of 3 GB. The installation time was the same in previous runs, but I expect it to improve in further pushes with the cache now generated

Edit: on another run, the Install Linux system dependencies step takes just over 1 minute, which is almost an 8x speedup over the previous 8 minutes required to install them! Not sure about the failing unit tests though, I will have a look

Since we are moving to a new release schedule in pybamm-team#2881. Caches once written will get dumped in seven days anyway and therefore get invalidated, so there might be no need for them here.
@agriyakhetarpal agriyakhetarpal changed the title 2447 cache Pythonic and non-Pythonic dependencies in CI 2447 cache Linux and macOS dependencies in CI May 20, 2023
@agriyakhetarpal
Copy link
Member Author

agriyakhetarpal commented May 20, 2023

@Saransh-cpp installing OpenBLAS explicitly and without cache helps pass the pybamm-requires step, but it fails the latexify and visualise.png unit tests – is there a way to make them optional?

Using the cache now manages to shave off 7-8 minutes when installing Linux dependencies, which is a significant boost (8-9 minutes comes down to 1.5 minutes). Assuming that the error is related to caching texlive-full, I tried installing OpenBLAS and texlive-full explicitly but it happens to reverse the benefits we gain completely. I will look into how we can cache texlive-full properly (it seems it takes the longest time to install amongst all the dependencies).

Anyhow; as reported by most Homebrew caching techniques I could find, it is unlikely to speed up our CI. Also, it is to be noted that the Homebrew cache size is exceeding actions/cache@v3's 5GB size limit every time – this might be the reason why the cache is being built at each workflow run (cache beyond the limit probably gets evicted) the actions/cache@v3 size limit is 10 GB per repository now regardless of the number of caches, so I reckon the cache is not being hit

@Saransh-cpp Saransh-cpp self-requested a review May 20, 2023 17:30
@agriyakhetarpal
Copy link
Member Author

I started working on this once more today since the issues with pybamm-requires and OpenMP have now been resolved upstream, and I think this PR is ready for review now and I shall mark it so. The caching here does have a few caveats and inconsistencies though, which I learned through my experience with several attempts and experiments with action runs to test it. Here is a summary:

  1. I tried an experimental solution for caching Homebrew packages by taking hints from workflows in other repositories, but the general accord is that it is unlikely to speed up our CI – this is because Homebrew formulae are small in size, so any caching solution with fast downloads from a CDN plus longer installation times would making caching redundant (the installation time will remain mostly the same), which is the case here. It is now taking extra time in the installation steps to cache Homebrew installation locations, for which I am unable to receive a proper cache hit.

    • The Homebrew environment variables are helpful and are nice-to-haves; they mostly speed up the command line by disabling analytics and cleanup processes (the latter happens every time an installation command succeeds).

    • We can probably remove the Homebrew caching altogether since it does not offer any real benefits?

  2. Caching with pip using the setup-python GitHub Action currently does not install the packages but only caches downloads (wheels), which is a known issue – PIP cache should cache the installed packages as well actions/setup-python#330. Also, most of our development workflow is dependent on tox (and will change soon to nox in Migrate from tox 3.28 to nox #3005), which means we would need to look for caching nox environments since almost all of our standard project-specific dependencies are installed within them.

    • Using actions/cache@v3 is not a solution either because the aforementioned action uses it under the hood. In previous runs with the cache enabled, setting Python up took ~5–10 seconds.

    • On the other hand, setting Python up without a cache does not take more than 3 seconds on any run, so pip caching is disadvantageous in its current state

  3. Caching apt packages with awalsh128/cache-apt-pkgs-action also causes problems with certain packages, especially ones that have pre-installation and post-installation steps/scripts (there is a parameter to execute_install_scripts but it is not foolproof):

    • I tried caching all packages first (gcc, gfortran, libopenblas-dev, and texlive-full) but it was quickly found that the pybamm-requires step failed because the runner had trouble finding BLAS libraries (I might need to set environment variables for its install location, but I'm not sure where exactly apt packages get installed on Linux). We might need to use ccache but I am unaware of an approach yet since we don't use CMake directly, we just use it through the install_KLU_sundials script that calls it.

    • It is to be noted that graphviz also has a post-installation step to set its fonts and plugins with dot -c after it is loaded from a previous cache.

    • I identified that the texlive-full package is one of the largest packages that get installed with more than 4 GB of occupied space (mostly because of its huge collection of extra fonts and documentation) which is not needed, so I replaced it with a medium-sized build of TeXLive, i.e., texlive-latex-extra and a few other packages such as divpng from CTAN. These packages are not included in this much smaller build (450 MB) but can be installed externally to account for missing features. texlive-full took ~8–10 minutes to collect its sub-dependencies and install every time, which now gets truncated to ~1 minute on average with texlive-latex-extra. Here is a comparison of different TeXLive builds and some dependency graphs for more explanation: https://tex.stackexchange.com/q/245982. I tried to cache TeXLive too, unsuccessfully (there were post-installation scripts for both texlive-full and texlive-latex-extra).

    • Nevertheless, the cache created without texlive-full (i.e., consisting of gcc, gfortran, and graphviz, since libopenblas-dev is installed separately) is barely 50 MB in size and does not offer any benefits either; there is a reduction of half a minute at most.

So this PR adds (i) a very minute speedup for macOS (brew) dependencies and (ii) a substantial speedup for Linux (apt) dependencies. I am glad to implement a cache for choco dependencies if required, though the only dependency on Windows is graphviz which gets installed fairly quickly.

@agriyakhetarpal agriyakhetarpal marked this pull request as ready for review June 6, 2023 09:30
Added in error earlier
Copy link
Member

@Saransh-cpp Saransh-cpp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks awesome, thanks for the detailed research on this, @agriyakhetarpal!

I went through the explanation above and it looks like you've sorted the Linux dependencies. We can skip the MacOS and Windows caching part for now (until there is a native GH Actions solution or a working third-party solution). We can also skip caching Python packages as the workflow currently only caches the wheel file.

Some very minor comments below -

agriyakhetarpal and others added 3 commits June 7, 2023 21:40
@agriyakhetarpal agriyakhetarpal changed the title 2447 cache Linux and macOS dependencies in CI 2447 cache Linux (apt) dependencies in CI Jun 7, 2023
@agriyakhetarpal
Copy link
Member Author

Thanks for the review! For Pythonic dependencies, we do have another solution via caching the entire Python virtual environment that I had shared earlier in the discussion (this blog) and since tox (or nox) also creates environments for installing PyBaMM and running the test suite, I will open another PR to try and cache the environments themselves after we migrate to nox in #3005. This way we can cache the lib/site-packages subfolders too, which should work in theory, and might save us some time since the dev and docs environments install a lot of packages.

@valentinsulzer valentinsulzer merged commit 82e7817 into pybamm-team:develop Jun 9, 2023
@agriyakhetarpal agriyakhetarpal deleted the 2447-cache-dependencies branch June 9, 2023 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cache apt packages in ci cache github actions
4 participants