choderalab / openmoltools

An open set of tools for automating tasks relating to small molecules
MIT License
63 stars 30 forks source link

Update for OpenMM namespace changes #304

Closed mikemhenry closed 3 years ago

mikemhenry commented 3 years ago

Fixes #303: There are changes happening in the openmm namespace, this should allow us to take advantage of the backwards compatible changes in the name space.

Also the README was pointing to the wrong CI badge, fixing that as well in this PR

mikemhenry commented 3 years ago

@jchodera Trying to fix the namespace issue but the errors I'm getting a weird, is this hydrogen = Element.getBySymbol("H") not the same hydrogen as

import simtk.openmm.app.element as element
element.hydrogen

That's the only thing I think that would be causing this that I have changed.

jchodera commented 3 years ago

I don't want to be the bottleneck in sorting this out---I'd work directly with @peastman and the OpenMM issue tracker to resolve this!

peastman commented 3 years ago

It should be exactly the same object.

>>> import openmm
>>> import simtk.openmm
Warning: importing 'simtk.openmm' is deprecated.  Import 'openmm' instead.
>>> simtk.openmm.app.element.hydrogen is openmm.app.element.hydrogen
True
>>> simtk.openmm.app.element.hydrogen is openmm.app.element.Element.getBySymbol('H')
True
>>> simtk.openmm.app.element.Element.getBySymbol('H') is openmm.app.element.hydrogen
True
mikemhenry commented 3 years ago

Okay so what I was doing isn't the source of the errors now in CI. When a repo doesn't have activity for 30 days, github stops running the cron CI jobs.

So the last time CI ran and passed was October 29th 2020 Then when I turned the CI cron jobs back on, things failed right away: https://github.com/choderalab/openmoltools/actions

It looks like the issue is some missing connect records in the pdb file. Since the code didn't change, the culprit is likely a dependency that updated.

mikemhenry commented 3 years ago

So I downgraded a few core packages where I thought this error could happen from

ambertools==20.9
openmm==7.4.0

And thought it worked since tests passed but really what was happening was:

test_amber.test_amber_binary_mixture ... SKIP: Skipping testing of packmol conversion because packmol not found.
test_amber.test_amber_box ... SKIP: Skipping testing of packmol conversion because packmol not found.
test_amber.test_amber_water_mixture ... SKIP: Skipping testing of packmol conversion because packmol not found.

So without the logs from the old CI runs, I have no idea if things were passing because things were working, or these tests were just skipped.

jchodera commented 3 years ago

Shoot, sorry about this. We just need to stabilize the bleeding here so we can finish ripping openmoltools out of our other projects.

mikemhenry commented 3 years ago

Exactly, I'm going to do the minimal amount possible to get nightly openmm to work. I think there is just one more thing to fix, which is why is parmed thinking the version of openmm installed isn't newer than 6.3 when it is the nightly

   File "/usr/share/miniconda/envs/test/lib/python3.8/site-packages/parmed/utils/decorators.py", line 30, in new_fcn
    raise ImportError('You must have at least OpenMM 6.3 installed')
ImportError: You must have at least OpenMM 6.3 installed

I can see some issues witht this logic: https://github.com/ParmEd/ParmEd/blob/3.4.1/parmed/utils/decorators.py

So I might need to look at what version of parmed is getting pulled in/install master for testing.

mikemhenry commented 3 years ago

Okay this is ready for review:

mikemhenry commented 3 years ago

@jchodera is there any other PRs or issues we should fix before cutting a release?

jchodera commented 3 years ago

@mikemhenry : If we're about to cut a new release, it would be good to squeeze in some deprecation warnings about force field generators that have migrated to openmm-forcefields: https://github.com/choderalab/openmoltools/issues/305