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

Gromacs 2018 and pytest low-performance custom plugin to control -nt #102

Merged
merged 12 commits into from
Aug 3, 2018

Conversation

ianmkenney
Copy link
Member

@ianmkenney ianmkenney commented Jan 27, 2017

Include Gromacs 2016 in tests (fixes #96)

EDIT: We use Gromacs 2018 (see PR #127 and discussion in #130 ). This PR contains experimental pytest plugin to provide the pytest --low-performance flag and code to explicitly set the number of threads for gmx mdrun in the tests.

@codecov-io
Copy link

codecov-io commented Jan 27, 2017

Codecov Report

Merging #102 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #102   +/-   ##
========================================
  Coverage    48.54%   48.54%           
========================================
  Files           24       24           
  Lines         4837     4837           
  Branches       733      733           
========================================
  Hits          2348     2348           
+ Misses        2331     2330    -1     
- Partials       158      159    +1
Impacted Files Coverage Δ
gromacs/utilities.py 53.52% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9416e07...ce9dbe8. Read the comment docs.

@orbeckst
Copy link
Member

orbeckst commented Feb 1, 2017

Similar problem with maxthreads as in Becksteinlab/MDPOW#77 ?

@orbeckst
Copy link
Member

orbeckst commented Feb 1, 2017

Can you enable full output so that we can see if this is again a problem with the number of threads? We probably need to explicitly restrict number of threads to 2 again...

@orbeckst
Copy link
Member

@ianmkenney can you have a look again, please?

I would like this to work.

@orbeckst
Copy link
Member

What is pytest-gmx and where is it used?

@ianmkenney
Copy link
Member Author

ianmkenney commented May 12, 2017 via email

@orbeckst
Copy link
Member

@ianmkenney can you move the repo to Becksteinlab so that all the GW-related things are together?

@orbeckst orbeckst force-pushed the enable-gromacs-2016 branch from bb4ccb6 to 753e3f9 Compare April 28, 2018 01:29
@orbeckst
Copy link
Member

orbeckst commented Apr 29, 2018

Rebased against develop but there are problems in the tests

@orbeckst orbeckst force-pushed the enable-gromacs-2016 branch 2 times, most recently from 01e4bff to 1ebb7d7 Compare April 30, 2018 09:15
@orbeckst
Copy link
Member

orbeckst commented Apr 30, 2018

rebased against develop (the errors in test_mdrun were fixed)

@orbeckst
Copy link
Member

orbeckst commented May 2, 2018

Btw, locally, the tests pass just fine with 2016.5.

@orbeckst
Copy link
Member

orbeckst commented May 2, 2018

Good and bad news:

  • Good: for the first time, all Gromacs 2016 tests run (probably due to using --low-performance, i.e., setting gmx mdrun -nt 2 instead of letting Gromacs choose the number of threads).
  • Bad: tests still fail
    • Gromacs 2016 tests (However, my local version of Gromacs 2016.5 passes the tests just fine.)
      • pdb2gmx generates a system with a non-integer charge (?!?!?!?!) – maybe check the occupancy fields of the PDB file. pdb2gmx complains about a few of them.
      • cg minimization fails with a constraint problem
    • Gromacs 4.6.7 tests: tests that used to pass, fail now:
      • cg energy minimization fails (not sure why)

@orbeckst orbeckst force-pushed the enable-gromacs-2016 branch from 010ee7c to 2a7651d Compare May 2, 2018 00:39
@orbeckst orbeckst changed the title Enable Gromacs 2016 in testing Gromacs 2016 and pytest low-performance custom plugin to control -nt May 16, 2018
@orbeckst orbeckst force-pushed the enable-gromacs-2016 branch from 2a7651d to b9c8d1b Compare July 31, 2018 01:21
@orbeckst
Copy link
Member

I rebased against develop (with #127 merged) and force pushed.

ianmkenney and others added 5 commits August 2, 2018 14:32
The new low_performance fixture (from https://github.com/ianmkenney/pytest-gmx
was not applied in derived classes. Fixed by removing superfluous methods
that just called the superclass method.

Also added a comment to the test that includes the low_performance fixture.
@orbeckst orbeckst force-pushed the enable-gromacs-2016 branch from b9c8d1b to 343e5b0 Compare August 2, 2018 21:35
@orbeckst orbeckst force-pushed the enable-gromacs-2016 branch from 48586b9 to 72f1ac5 Compare August 2, 2018 22:02
@orbeckst orbeckst changed the title Gromacs 2016 and pytest low-performance custom plugin to control -nt Gromacs 2018 and pytest low-performance custom plugin to control -nt Aug 2, 2018
@orbeckst
Copy link
Member

orbeckst commented Aug 2, 2018

This looks good, and the run time of the mdrun tests decreased from ~20s to ~4s by setting -nt 2. Will merge.

@orbeckst orbeckst force-pushed the enable-gromacs-2016 branch from 2f4b440 to 41724a1 Compare August 2, 2018 23:30
@orbeckst orbeckst force-pushed the enable-gromacs-2016 branch from 41724a1 to 905f091 Compare August 2, 2018 23:33
- Gromacs 2018 insists on -r for grompp, earlier uses the structure as default
- required to make more Gromacs 2018 tests pass
- makes 2018 tests pass
- small code cleanup in tests/top/top.py
- Gromacs 2018 on Python 2.7 passes all tests
- fix #96
- Python 3.6 tests may fail (see issue #132)
- updated CHANGES
@orbeckst orbeckst force-pushed the enable-gromacs-2016 branch from 121db79 to f9e0dde Compare August 3, 2018 00:33
needed so that it can be used with all fixtures
- test does not always hit the convergence criterion and then errors; this seems stochastic
  (eg depends on architecture); rerun the test with a much higher tolerance once this happens
  and hope for the best
- use the low_performance fixture to set nt=2 for test_setup.py
@orbeckst orbeckst self-assigned this Aug 3, 2018
@orbeckst orbeckst merged commit 6b87aa8 into develop Aug 3, 2018
@orbeckst orbeckst deleted the enable-gromacs-2016 branch August 3, 2018 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants