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

Run tests in parallel #1798

Merged
merged 35 commits into from
Sep 4, 2024
Merged

Run tests in parallel #1798

merged 35 commits into from
Sep 4, 2024

Conversation

abhisrkckl
Copy link
Contributor

@abhisrkckl abhisrkckl commented Jul 2, 2024

@abhisrkckl abhisrkckl changed the title Run tests in parallel WIP: Run tests in parallel Jul 2, 2024
@abhisrkckl
Copy link
Contributor Author

The test failures seem to be due to astropy cache issues. #1374 may be a solution.

@abhisrkckl abhisrkckl changed the title WIP: Run tests in parallel Run tests in parallel Aug 29, 2024
@abhisrkckl
Copy link
Contributor Author

abhisrkckl commented Aug 29, 2024

Here is a summary of what I am doing:

  1. Parallelize tests using pytest-xdist. I am using 6 processes here. This is greater than the number of CPU cores available (4), but that's OK because many of our tests are IO-limited rather than CPU-limited. I am getting ~2-3x improvement in test execution time.
  2. Re-run failed tests using pytest-rerunfailures. This handles intermittent failures that occur due to random chance.
  3. In the macos test, test_toa_selection.py keeps failing in the parallel mode (but is OK when run serially). So I am running that test serially and everything else parallelly for macos. This doesn't affect coverage because we are not computing coverage for macos.
  4. I have also updated some tests to use fixtures rather than global variables (these are needed for parallel execution).

@abhisrkckl
Copy link
Contributor Author

abhisrkckl commented Aug 29, 2024

Old Run times (approx)

ubuntu-latest, oldestdeps - 35 min
macos-12, py312-test - 45 min
ubuntu-latest, py312-test-cov - 70 min

New run times (approx)

ubuntu-latest, oldestdeps - 15 min
macos-12, py312-test - 30 min
ubuntu-latest, py312-test-cov - 30 min

These times are for the github runners. They run a lot faster on my laptop.

@abhisrkckl abhisrkckl added the awaiting review This PR needs someone to review it so it can be merged label Aug 29, 2024
@dlakaplan
Copy link
Contributor

This is good. I remember we had tried this a while ago but for some reason didn't implement it in CI. But I don't see any reason not to. Even a factor of ~2 change helps.

Is this ready to merge?

@abhisrkckl
Copy link
Contributor Author

Yes

@dlakaplan dlakaplan merged commit 1f295d6 into nanograv:master Sep 4, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting review This PR needs someone to review it so it can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants