openforcefield / openff-qcsubmit

Automated tools for submitting molecules to QCFractal
https://openff-qcsubmit.readthedocs.io/en/latest/index.html
MIT License
26 stars 4 forks source link

Remove `openmm` as a dependency, switch to `openff.units`? #239

Closed dotsdl closed 6 months ago

dotsdl commented 8 months ago

Currently, there are a handful of uses of openmm in QCSubmit, but these appear to only:

  1. use openmm.app.Element to convert from symbols to atomic numbers, and vice-versa.
  2. use openmm.unit for units.
  3. perform a conversion to an openmm topology, only to convert this to an mdtraj topology to perform Baker-Hubbard hydrogen bond filtering.

I believe (1) and (2) can be addressed by switching to the use of openff.units, though (3) may not be easy, since even the openff-toolkit Topology._to_mdtraj method goes through openmm as well.

If we could remove OpenMM as a direct dependency, this would avoid the need for environments such as our qca-dataset-submission automation actions to pull in cudatoolkit, which is rather large and very unnecessary.

mattwthompson commented 8 months ago

Definitely!

Which version are you looking at? (2) is meant to already be in place. (1) definitely should be going through openff.units. I think moving OpenMM to an optional dependency only used for HydrogenBondFilter could be a stopgap for (3), but a better solution might involve going to MDTraj without OpenMM as an in-between or using something other than MDTraj. At a surface level, qca-dataset-submission does not need this filter in its deployment, from which I gather the filter is something that's more user-facing?

(CPU-only OpenMM builds would also be nice, but substantially more work AFAIR.)

mattwthompson commented 8 months ago

Working on this in #238

dotsdl commented 8 months ago

Oh damn, just saw #237 merged two days ago. :sweat_smile:

So yes, (2) should be good! And (1) would solve the issue we're hitting in the qca-dataset-submission automation: https://github.com/openforcefield/qca-dataset-submission/pull/333#issuecomment-1802399002

And you're right, the HydrogenBondFilter bits shouldn't impact us on qca-dataset-submission!

mattwthompson commented 6 months ago

Resoled in #238, release unknown