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

Use pytest-split to parallelize across 3 runners and speedup CI #985

Merged
merged 127 commits into from
Sep 29, 2024

Conversation

esoteric-ephemera
Copy link
Contributor

@esoteric-ephemera esoteric-ephemera commented Sep 16, 2024

Summary

Splits atomate2 tests into batches using pytest-split (see related pymatgen change a while ago). Test times were generated from a CI run and averaging over python versions. This PR is synced up with #940 per the discussion there. Copying @janosh

Current timing info

The forcefield Gruneisen workflow previously took a very long time, ~845 sec in CI. Making some small tweaks to its BasePhononMaker has reduced the time significantly to about 130 sec. Now the tests run at 310 sec/split for 3 splits

Previous timing info

The best load balancing is probably 2-3 splits, since one workflow (forcefield Gruneisen) takes a very large amount of time

Some splits vs. timing notes:

  • 3 splits: 1 split only runs 1 test (845 sec/split, the forcefield Gruneisen wf), the remaining 2 run all other tests for an average of 400 sec/split
  • 4 splits: 1 split only runs 1 test (845 sec/split, the forcefield Gruneisen wf), the remaining 3 run all other tests for an average of 265 sec/split
  • 5 splits: 1 split only runs 1 test (845 sec/split, the forcefield Gruneisen wf), the remaining 4 run all other tests for an average of 195 sec/split

@janosh janosh added dx Developer experience ci Continuous integration labels Sep 27, 2024
Copy link
Member

@janosh janosh left a comment

Choose a reason for hiding this comment

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

thanks a lot @esoteric-ephemera! 👍 i resolved the merge conflicts. looks ready to go

@janosh janosh enabled auto-merge (squash) September 27, 2024 18:29
@janosh janosh changed the title Split tests Use pytest-split to parallelize across 3 runners and speedup CI Sep 27, 2024
@janosh janosh merged commit ec1b598 into materialsproject:main Sep 29, 2024
15 checks passed
@utf utf added house-keeping Formatting and code quality tweaks testing Test all the things and removed house-keeping Formatting and code quality tweaks testing Test all the things labels Sep 30, 2024
hrushikesh-s pushed a commit to hrushikesh-s/atomate2 that referenced this pull request Nov 16, 2024
* move ase tests to separate test run, use pytest-split on rest of tests, 3 runs per linux version

* add test durations for split

* update test split, run notebook test as separate test, update test time

* move jupyter notebook test into ase

* tweak some clean_dir to tmp_dir to prevent unncessary test file creation

* reduce to 4 splits

* go back to 3 splits

* try to get better ci timing estimate per @janosh's suggestion

* fix test split cmd

* revert pytest split change for separate pr

* sync with ase branch and add test split logic / times

* tweak wf

* change pytest split algo, see if 5 splits works better for balancing

* change to 3 splits

* change gruneisen test for time use - just make phonon maker cheaper

* add ase to phonon supported codes, enforce string literal in BasePhononMaker

* update timing for gruneisen

* try to fix ci test wf

* ci test wf

* merge conflict fixes, try reorg tests

* pcmt, other tweaks

* remove lingering ase frechet filter remarks

---------

Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous integration dx Developer experience house-keeping Formatting and code quality tweaks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants