Converter for OpenMM

[ahy3nz]

Partially addresses #2863 and #2367

Changes made in this Pull Request:

• Add topology converters for openmm Simulation, PDBFile, Modeller, PDBxFile, and Topology objects
• Add coordinate converters for openmm Simulation, PDBFile, Modeller, and PDBxFile objects
• Tests included

Openmm has a lot of classes that wrap fileformats, but they all have properties for positions (numpy arrays) and topologies (openmm topology). I'm not sure what the "best" way to handle this is, but I went with trying to create one mdanalysis reader class for each openmm object (they end up re-using a lot of functions). Alternatively, could you create a universe from a numpy array of positions + the openmm topology? (if you can, I don't know how) This could be a uniform pattern for converting a lot of openmm objects into mdanalysis universe objects while reducing the number of reader classes.

When testing the openmm Topology reader, I had to specify the topology_format flag -- do the _READERS only infer for coordinate readers?

There's been nothing done with trying to infer angles/dihedrals/etc from the forces within an openmm system. There's also been nothing done with trying to create openmm objects from mdanalysis objects

I tried to adjust some yml files that install from conda. I don't think openmm is available on pip

PR Checklist

• Tests?
• Docs?
• CHANGELOG updated?
• Issue raised/referenced?

[pep8speaks]

Hello @ahy3nz! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file package/MDAnalysis/coordinates/OpenMM.py:

Line 29:80: E501 line too long (145 > 79 characters)
Line 34:80: E501 line too long (80 > 79 characters)
Line 36:80: E501 line too long (165 > 79 characters)
Line 37:80: E501 line too long (171 > 79 characters)
Line 38:80: E501 line too long (171 > 79 characters)
Line 43:80: E501 line too long (90 > 79 characters)
Line 105:80: E501 line too long (81 > 79 characters)
Line 115:17: E128 continuation line under-indented for visual indent
Line 141:80: E501 line too long (205 > 79 characters)
Line 159:17: E128 continuation line under-indented for visual indent
Line 171:80: E501 line too long (81 > 79 characters)
Line 194:80: E501 line too long (110 > 79 characters)

• In the file package/MDAnalysis/topology/OpenMMParser.py:

Line 31:80: E501 line too long (137 > 79 characters)
Line 32:80: E501 line too long (85 > 79 characters)
Line 37:80: E501 line too long (165 > 79 characters)
Line 38:80: E501 line too long (183 > 79 characters)
Line 39:80: E501 line too long (171 > 79 characters)
Line 40:80: E501 line too long (171 > 79 characters)
Line 42:80: E501 line too long (86 > 79 characters)
Line 189:1: W391 blank line at end of file

• In the file testsuite/MDAnalysisTests/coordinates/test_openmm.py:

Line 166:80: E501 line too long (114 > 79 characters)

Comment last updated at 2021-05-05 15:48:33 UTC

[orbeckst]

Hi @ahy3nz thanks for giving this one a try. Please be a little patient with the core devs... we're at the end of GSoC and most of the time for reviewing PRs currently goes into getting GSoC PRs wrapped up. You can also email the developer list with questions, sometimes that gets a quick reply, especially if one can answer in a sentence or two. But rest assured, you're not being ignore, we're just stretched thin!

[orbeckst]

@ahy3nz if you need some additional eyes or have questions, feel free to ping; @IAlibay @lilyminium (and possibly others) were also interested in OpenMM integration.

[ahy3nz]

@lilyminium @IAlibay @orbeckst @jbarnoud Please let me know what else needs to be done. Otherwise, if this is currently an undesired feature/approach, I can go ahead and close this PR

[IAlibay]

@ahy3nz this is definitely still a very much desired feature.

I'm so sorry about the delay here, the main issue we're facing is that we have a crazy large queue of fixes to do before we can even start concentrating on v2.0 stuff + lots of GSoC stuff to deal with :(

If one of the @MDAnalysis/coredevs has capacity to take over the review here that would be great!

I'll put it as a milestone for 2.0, so hopefully that'll get someone to finish up the review ASAP.

[lilyminium]

Thanks @ahy3nz and so sorry for the long delay. I'm not super familiar with OpenMM so I have some questions about units and the PDB angle parsing.

[lilyminium]

Also if you could fix the merge conflicts and PEP8 as noted by the bot (#2917 (comment)), that'd be great.

[jbarnoud]

I am looking at this PR now. I tested it on my use case and it worked neatly! I still need to understand why codecov reports numbers so low for the test coverage while I do see quite a lot of tests. I will also check how the doc looks and fix for PEP8 where relevant.

[jbarnoud]

The low coverage was because of the restrictions github recently added to the CI. I allowed the workflows to run and the patch coverage is 98.43%.

The docstrings do not appear in the documentation and the linter still complains about some PEP8 errors. Instead of asking @ahy3nz to grind through a bunch of tiny trivial fixes, I opened a PR against their branch: ahy3nz#1

With these small fixes, the PR looks good to go for me.

[jbarnoud]

Note that it would be very cool to have this merged in for version 2.0. The feature freeze for that version is on Friday so it would be great if you could merge my changes by then. Sorry to rush you after I made you wait for basically ever, but the feature really is great!

[IAlibay]

@ahy3nz this is your first PR for MDAnalysis right? Please don't forget to add yourself to AUTHORS and update the CHANGELOG too! 🎉

edit: ignore me, for some reason my github view didn't include all the files.

[jbarnoud]

Looks good to me 🚀

[IAlibay]

I'm not super acquainted with the inner workings of OpenMM, but lgtm

All the comments are addressed, so I dismiss the review with @lilyminium approval.

[jbarnoud]

Congratulations on your first contribution @ahy3nz! It as a substantial one and I am very glad it is in. I am very sorry it took so long.