mdanalysis
mdanalysis copied to clipboard
Limit threads usage in numpy during test to avoid time-out
fix #4209
Changes made in this Pull Request:
- make numpy only use one thread during test
- reduce pytest worker
- make tests lighter
Similar to https://github.com/MDAnalysis/mdanalysis/pull/2950, allowing numpy to make full usages of the resources clogs the machine and likely leads to the ~~occassional~~ frequent time-out in multiprocessing-related testings.
See the comparison between duration below; it takes a lot more time in test_encore for numpy calculations before.
new test duration
============================= slowest 50 durations =============================
11.82s call MDAnalysisTests/analysis/test_contacts.py::TestContacts::test_n_initial_contacts[datafiles1-41814]
10.93s call MDAnalysisTests/coordinates/test_gro.py::TestGROLargeWriter::test_writer_large
10.70s call MDAnalysisTests/coordinates/test_pdb.py::test_conect_bonds_all
9.34s call MDAnalysisTests/parallelism/test_multiprocessing.py::test_multiprocess_COG[u11]
9.02s call MDAnalysisTests/utils/test_pickleio.py::test_fileio_pickle
8.53s call MDAnalysisTests/analysis/test_gnm.py::test_closeContactGNMAnalysis
8.31s call MDAnalysisTests/analysis/test_gnm.py::test_closeContactGNMAnalysis_weights_None
8.27s call MDAnalysisTests/analysis/test_encore.py::TestEncore::test_ces_error_estimation_ensemble_bootstrap
7.62s call MDAnalysisTests/coordinates/test_gro.py::TestGROLargeWriter::test_write_trajectory_universe
7.52s call MDAnalysisTests/coordinates/test_gro.py::TestGROLargeWriter::test_write_trajectory_atomgroup
old test duration (https://github.com/MDAnalysis/mdanalysis/actions/runs/8869719078/job/24350703650)
20.06s call MDAnalysisTests/analysis/test_encore.py::TestEncore::test_hes_to_self
18.93s call MDAnalysisTests/analysis/test_encore.py::TestEncore::test_hes
13.07s call MDAnalysisTests/analysis/test_encore.py::TestEncore::test_ces_error_estimation_ensemble_bootstrap
12.93s call MDAnalysisTests/analysis/test_encore.py::TestEncore::test_hes_custom_weights
12.80s call MDAnalysisTests/coordinates/test_pdb.py::test_conect_bonds_all
12.12s call MDAnalysisTests/analysis/test_contacts.py::TestContacts::test_n_initial_contacts[datafiles1-41814]
11.40s call MDAnalysisTests/analysis/test_gnm.py::test_closeContactGNMAnalysis
10.79s call MDAnalysisTests/coordinates/test_gro.py::TestGROLargeWriter::test_writer_large
gonna run it three times.
pass once
pass twice
PR Checklist
- [ ] Tests?
- [ ] Docs?
- [ ] CHANGELOG updated?
- [ ] Issue raised/referenced?
Developers certificate of origin
- [ ] I certify that this contribution is covered by the LGPLv2.1+ license as defined in our LICENSE and adheres to the Developer Certificate of Origin.
📚 Documentation preview 📚: https://mdanalysis--4584.org.readthedocs.build/en/4584/
Linter Bot Results:
Hi @yuxuanzhuang! Thanks for making this PR. We linted your code and found the following:
Some issues were found with the formatting of your code.
Code Location | Outcome |
---|---|
main package | ✅ Passed |
testsuite | ⚠️ Possible failure |
Please have a look at the darker-main-code
and darker-test-code
steps here for more details: https://github.com/MDAnalysis/mdanalysis/actions/runs/10782714307/job/29903235743
Please note: The black
linter is purely informational, you can safely ignore these outcomes if there are no flake8 failures!
Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 93.84%. Comparing base (
b3208b3
) to head (555520a
).
Additional details and impacted files
@@ Coverage Diff @@
## develop #4584 +/- ##
===========================================
- Coverage 93.87% 93.84% -0.04%
===========================================
Files 173 185 +12
Lines 21428 22494 +1066
Branches 3980 3980
===========================================
+ Hits 20116 21109 +993
- Misses 858 931 +73
Partials 454 454
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hello @yuxuanzhuang! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:
- In the file
testsuite/MDAnalysisTests/analysis/test_encore.py
:
Line 128:53: E231 missing whitespace after ','
Comment last updated at 2024-09-09 23:10:45 UTC
@orbeckst @IAlibay I now believe that these three adjustments can reduce the chance of timeouts:
-
Limiting the number of threads used by numpy during testing. This also decreases the duration of tests that are heavily dependent on numpy and computationally intensive, e.g. in encore.
-
Reducing the number of workers in pytest to prevent overloading the entire test runner.
-
Lightening the tests. I have already modified some, but we can certainly further optimize the other time-consuming tests.
I have re-run the GitHub CI three times consecutively, and they all pass. I have also patched it to #4162 that are more likely to time-out in https://github.com/yuxuanzhuang/mdanalysis/pull/6 and it also seems to work well.
@IAlibay would you please shepherd the PR to completion?
@IAlibay with my PR shepherding hat on, just pinging here, no rush though :smile:
@IAlibay just pinging here with my review coordination hat on.
Since this PR doesn’t really resolve the time-out issue, I will repurpose it to focus on reducing the test duration.