⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.02%. Comparing base (5d0c607) to head (936291a).
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##             main     #392      +/-   ##
==========================================
+ Coverage   71.41%   81.02%   +9.61%     
==========================================
  Files           5        5              
  Lines         822      822              
==========================================
+ Hits          587      666      +79     
+ Misses        235      156      -79     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@j-wags Because we were not running the espaloma tests on CI, we missed that this if is also needed for the espaloma system generator. Instead of copying the fix over to the espaloma system generator, I opted to move the fix to convert_system_to_ffxml since that is the method that ends up giving us the error with the CMMotionRemover. This way any future system generator templates that use ffxmls will get this fix as well.

Thanks for getting these running!

It would be nice to test against OpenMM 8.3.1. (Not sure if we would want to include it along with both 8.1.2 and 8.2.0, or drop 8.1.2 and keep testing only the most recent 2 minor releases.) I had tried this in another PR I was working on and it was OK, but it might be worth seeing if this is possible here or if we'd run into any dependency conflicts with espaloma.

It would be nice to test against OpenMM 8.3.1

I'll make a separate PR for that, I think it would be good to drop python 3.10 and add 3.13, and I think it makes sense to test 8.3.0 and 8.3.1 (keeping with the tradition of testing the most recent 2 releases of openmm)

but it might be worth seeing if this is possible here or if we'd run into any dependency conflicts with espaloma

I was worried about that as well, which is why I am thinking about doing this in a separate PR since it might require more work

If we merge in #394 first then we can update this PR and check to see if everything still works okay.

I still see 3.10 checks, which I don't like to see

I still see 3.10 checks, which I don't like to see

They are not running, the branch protection UI is being weird with what names it wants

I think there is a hole in the numpy/dgl/pytorch/python/openmm builds for python 3.13 with espaloma, once I confirm that I will set things up so we skip the espaloma tests for python 3.13

okay fixed which checks need to be required

BTW, the root cause is there are no python 3.13 builds of DGL.

Good to know. I think it's fine then if we can make the espaloma test runs conditional on the Python version, as that would be better than not being able to run them at all.