search

openmm / openmmforcefields

CHARMM and AMBER forcefields for OpenMM (with small molecule support)
http://openmm.org
Other
250 stars 80 forks source link

Improve `espaloma` Support #336

Open mikemhenry opened 4 months ago

mikemhenry commented 4 months ago

We should update the default espaloma ff to espaloma-0.3.2.pt

ijpulidos commented 4 months ago

Espaloma 0.3.x is supported, we have run simulations using it for both protein and small molecules. That said, the way to use it is probably not that easy to accomplish, we should make this more streamlined.

We had specific GHA workflows to test the espaloma support but I don't know why or when these were removed. Sounds like we should get these back.

mattwthompson commented 4 months ago

I'm curious the use cases of Espaloma with this package vs. just using Espaloma's API directly to generate parameters (in the form of an openmm.System)? I can only think of

The current architecture of building something on top of SystemGenerator (which itself has a lot of magic built on top of these templates) which parses Espaloma data to eventually get a system - compared to just getting it directly from Espaloma's parameters. It may be that rewriting downstream code to use Espaloma's API is not so simple, I'm not sure

mikemhenry commented 4 months ago

They were just consolidated into a single CI file, I said "improve" espaloma support since for the end-user it looks like we only support once espaloma ff, I will update the issue instead to update what we consider the "default" espaloma version (just like we updated the default openff version)

https://github.com/openmm/openmmforcefields/blob/f94397947d42f53ab8717349220f999a75a7a584/openmmforcefields/generators/template_generators.py#L1801-L1807

ijpulidos commented 4 months ago

They were just consolidated into a single CI file, I said "improve" espaloma support since for the end-user it looks like we

Ah I see, what I saw is that the espaloma tests are being skipped. I set these tests to be run with the --runespaloma flag, we should probably add that to the pytests flags or remove the requirement for it. At the time this was designed in such a way because the espaloma tests were optional and we only wanted to run the espaloma tests in that specific CI workflow.

ijpulidos commented 4 months ago

I'm curious the use cases of Espaloma with this package vs. just using Espaloma's API directly

@mattwthompson This is a good point to raise. As far as I can tell, we do want to maintain the possibility of using Espaloma only for specific portions of the topology (be it only the small molecule, or a mutant residue, a cofactor, etc.). However, I agree that the current approach is very cumbersome and hard to maintain in the longer term.