-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Replace ParmEd with Interchange in Amber/GROMACS example #1113
Conversation
mattwthompson
commented
Oct 27, 2021
•
edited
Loading
edited
- Re-reun once Interchange v0.1.2 is online
- Tag issue being addressed
- Add tests
- Update docstrings/documentation, if applicable
- Lint codebase
- Update changelog
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we endorse Interchange for use in small molecules, but not proteins (yet), it'd be good to keep the old ParmEd notebook around. So maybe here we could:
- Rename the original notebook to be something like
convert_to_amber_gromacs_using_parmed
- Name the new notebook something like
convert_amber_gromacs_lammps_using_interchange
- Add a big notice at the top of each
- ParmEd notebook: "Please take a look at the other notebook in the folder, showcasing how to use OpenFF interchange. Interchange is well-validated for small molecule conversion to several formats"
- Interchange notebook: "We're working on replacing use of ParmEd with interchange... It works for small molecules but we expect there may be issues for larger things, we'd love feedback..."
I can't reproduce the test failures locally, even with downgrading to Python 3.7 and in a similar-ish-looking environment: https://gist.github.com/mattwthompson/eaef9191b31ce891816acfdb6e93f50e |
Yeah, QCArchive seems to be down/unreachable. MolSSI is aware and they're working on resolving. |
I'm getting errors from Sander in these tests, though:
which do not occur locally. |
Ahh, right, those should be independent from the QCA issues. Sorry - my brain is booting up slowly today. I'm looking into what's happening here. |
Hm, one commit ago it was failing to find I'm also noticing that the new notebook PASSES on macos with RDKit only. Here's the breakdown:
|
No worries! I haven't implemented great error handling yet, since sometimes engines yell at me via STDOUT, sometimes via STDERR, and sometimes they're quiet and hide errors in a file. If you or somebody else has time, It would be useful to see if this can be reproduced locally; if you can't, then I'll have to work on error handling and come back to this later. |
The code to find My personal experience has always been that having AmberTools installed also provides Sander. I've never had issues with LAMMPS or GROMACS installs conflicting ... oh, now I see that the non-RDKit builds forcefully remove AmberTools. 🤦 That'll explain at least two of the three errors in the matrix. |
Does our distribution of examples as a conda package mean we can only have |
Good catch. Not quite. We already have a GROMACS section in the "appendix" of the |
I'm running out of resources tracking down this behavior; I've gone so far as printing out the files themselves and nothing seems out of order. They pass the eye check. I can copy+paste them into a new folder and run them just fine, both directly and via Click to expand!
|
Nothing stands out to me in the AmberTools changelog: https://ambermd.org/ATPatches.php |
Here are some possibilities I can think of:
|
Lily suggested running the notebook as a script - with a couple steps (
If it fails in CI as a script, there is something wrong with the actual code and I'll need to revisit how I'm using subprocesses. If not, there is something funky happening with the notebook and/or how it's called from |
So it looks like the script passes with ambertools on Mac, but not Ubuntu. I took the liberty of restarting the jobs to see if this would happen again; if so, it implies there's something up with the code and the Jupyter format. Or possibly it's all pytest acting up and not freeing memory properly, although if the CI passes with sander calls then that's probably not it. Ubuntu ambertools (failing): ambertools 21.11 py39hc630cb1_0 conda-forge |
Thanks again for helping with this. The way I call other programs is relatively naive, but it's pretty boilerplate and hasn't given me much trouble in the past: The system is small (two molecules) so hardware limits shouldn't be the issue. Memory leaking at some point in the process (an executable starting tests, which calls something else to run the notebook, which then further spawns a new subprocess) sounds totally plausible, if hard to hunt down. I'm updating an old Linux box to see if I can reproduce it locally. Some obscure OS difference would explain why it works for me locally (normally using macOS) but fails on CI. |
I can reproduce this on an machine running Linux, and one fix is to remove the box information from the coordinate file. I'll need to gather information on if people want box information in both the coordinate and topology file. |
pip install git+https://github.com/openforcefield/openff-interchange.git@skip-drivers | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To be explicit: I will include this branch into a release before aiming to have this PR merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is now included in v0.1.3; CI will need to be restarted next week to get it to pass (the package will not be online until after hours)
The issue, or at least what I fixed locally to get it working, is this: openforcefield/openff-interchange@6e4fd12 |
Failures in last run are codecov. |
…nal dependencies (#1142) * Add openff-units to test environments * Make OpenMM optional in most of topology and typing modules * More fixes in Molecule * Improve parsing of SMIRNOFF XML source * More fixes for force field tests * Test against openff-units/smirnoff-per-mol * Fix energy tests * Patch some unit interoperability in utils/utils.py * Fixes for charge assignment tests * Let some Molecule setters take in openmm.unit quantities * [pre-commit.ci] pre-commit autoupdate (#1119) updates: - [github.com/psf/black: 21.9b0 → 21.10b0](psf/black@21.9b0...21.10b0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Bump actions/checkout from 2.3.5 to 2.4.0 (#1121) Bumps [actions/checkout](https://github.com/actions/checkout) from 2.3.5 to 2.4.0. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v2.3.5...v2.4.0) --- updated-dependencies: - dependency-name: actions/checkout dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Add 404 page to docs (#1115) * Add 404 page * Use sphinx-notfound-page in RTD but skip building the 404 page if not installed * Improve language in 404 * Add link to stable version on 404 page * Relocate the SMIRNOFF spec to standards (#949) * Relocate the SMIRNOFF spec to standards * Update links to SMIRNOFF spec in standards * Update links in release history Co-authored-by: Matthew W. Thompson <mattwthompson@protonmail.com> Co-authored-by: Josh Mitchell <yoshanuikabundi@gmail.com> Co-authored-by: Jeff Wagner <jwagnerjpl@gmail.com> * [pre-commit.ci] pre-commit autoupdate (#1124) updates: - [github.com/PyCQA/isort: 5.9.3 → 5.10.0](PyCQA/isort@5.9.3...5.10.0) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Use pytest-rerunfailures on tests that rely on QCArchive (#1116) * Use pytest-rerunfailures on tests that rely on QCArchive * Update openff/toolkit/tests/test_molecule.py Co-authored-by: Jeff Wagner <jwagnerjpl@gmail.com> Co-authored-by: Jeff Wagner <jwagnerjpl@gmail.com> * [pre-commit.ci] pre-commit autoupdate (#1127) updates: - [github.com/PyCQA/isort: 5.10.0 → 5.10.1](PyCQA/isort@5.10.0...5.10.1) Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> * Make FrozenMolecule.to_hill_formula a class method (#1120) * Make FrozenMolecule.to_hill_formula a class method * Skip Hill formula short-circuit in isomorphism check * Remove debug code * Support some amount of duck-typing in isomorphism check * Fix typo in test * Fix type annotation * Add automated tests against OpenMM/OpenEye RCs (#1111) * Add automated tests against OpenMM/OpenEye RCs * Rework to unconditionally test against all RC/beta builds * Run on push * Fix syntax * Make OE RC higher priority than OE beta * Remove dev code * Update beta_rc.yaml * Use strict channel priority * Switch back to debug * Remove dev trigger * Drop OpenEye beta channel * Replace ParmEd with Interchange in Amber/GROMACS example (#1113) * Add Interchange + GROMACS to existing GROMACS example * Overhaul Amber + GROMACS example * Update conda env used by binder * Refactor protein-ligand examples to not use ParmEd * Add back ParmEd-based example * Fix other files * Rename notebooks * Add header notes to each notebook * add new example deps to example yaml * Skip Interchange example when AmberTools is not installed * Do not assume LAMMPS and GROMACS are installed * Fix how GROMACS/Amber notebook is detected * Debug files * Move debug so it might get printed * Split out Amber/GROMACS example into earlier step * Run examples with Python 3.9 * try not using mamba in case its causing trouble with ambertools * re enable mamba, force ambertools downgrade * Try running it as a test script * Unpin AmberTools * Revert some debugging in workflow * Remove script used for debugging * Pin to interchange v0.1.3+ * Update CHANGELOG Co-authored-by: Jeff Wagner <jwagnerjpl@gmail.com> * Fix examples workflow (#1128) * Simplify virtual site serialization, fix some tests * Fix tmp_path usage * Bump actions/setup-python from 2.2.2 to 2.3.0 (#1131) Bumps [actions/setup-python](https://github.com/actions/setup-python) from 2.2.2 to 2.3.0. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v2.2.2...v2.3.0) --- updated-dependencies: - dependency-name: actions/setup-python dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Fix tmp_path usage (#1130) * Fix tmp_path usage * Update release history * Fixes for topology unit tests * Fixes for test_molecule * Fixes for test_toolkits*py * Fixes for test_utils * Fix units of virtual site positions * [pre-commit.ci] pre-commit autoupdate (#1134) * [pre-commit.ci] pre-commit autoupdate updates: - [github.com/psf/black: 21.10b0 → 21.11b1](psf/black@21.10b0...21.11b1) - [github.com/nbQA-dev/nbQA: 1.1.1 → 1.2.1](nbQA-dev/nbQA@1.1.1...1.2.1) * Have pre-commit.ci run autoupdate monthly Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Matthew W. Thompson <mattwthompson@protonmail.com> * Fix fstrings for Python 3.7 compatibility * Fixes for GBSA tests * Fix test_tip5p_dimer_energy * Style fixes * Fixes for some parameter assignment tests * Install released version of openff-units * Fix GLOBAL_TOOLKIT_REGISTRY etc being expanded in sigs (#1135) * Bump actions/setup-python from 2.3.0 to 2.3.1 (#1138) Bumps [actions/setup-python](https://github.com/actions/setup-python) from 2.3.0 to 2.3.1. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@v2.3.0...v2.3.1) --- updated-dependencies: - dependency-name: actions/setup-python dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Update examples to use openff-units * Streamline CI github actions (#1125) * matrixify CI * fix list * Revert "fix list" This reverts commit 16cad1c. * redo packages * check uninstalled toolkits * tidy up and rm comment * Fix Mypy * Remove unused imports * Update FAQ.md (#1139) * Update FAQ.md * Apply suggestions from code review Co-authored-by: Matt Thompson <mattwthompson@protonmail.com> * Apply suggestions from code review Co-authored-by: Josh A. Mitchell <yoshanuikabundi@gmail.com> * link to core concepts when talking about what an OFFMol needs to know * fix core concepts link Co-authored-by: Matt Thompson <mattwthompson@protonmail.com> Co-authored-by: Josh A. Mitchell <yoshanuikabundi@gmail.com> * Fix typing tests * Switch to mendeleev * Drop simtk support * Update dependencies * Mark failing tests as flaky * More forceful xfail * Make entire openmm module optional * Add mendeleev to test envs * Fix imports * Fix merge conflict * Fix imports * Fix mypy Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Josh A. Mitchell <yoshanuikabundi@gmail.com> Co-authored-by: SimonBoothroyd <simon.boothroyd@hotmail.com> Co-authored-by: Jeff Wagner <jwagnerjpl@gmail.com> Co-authored-by: Lily Wang <31115101+lilyminium@users.noreply.github.com>