search

choderalab / feflow

Recipes and protocols for molecular free energy calculations using the openmmtools/perses and Open Free Energy toolkits
MIT License
10 stars 1 forks source link

Charge handling -- Partial charges settings and charge transformation support #49

Open ijpulidos opened 3 weeks ago

ijpulidos commented 3 weeks ago

This set of changes aim to support charge transformations (up to one unit of charge changes) and give the users the ability to specify the partial charge assigning method via a friendly settings object. This is based on a lot of the functionality already included in the RelativeHybridTopologyProtocol in the openfe package, with some minor modifications.

Solves #19 Solves #47 Closes #20

ijpulidos commented 3 weeks ago

This adds a charge.py module to the utils subpackage. This might be open to suggestion on whether having such module and utilities is the better way of doing this. I just found a little confusing the way these things are organized on the openfe repo, since there are things in different places. I just found convenient to have them in a single module/place as possible, if that makes any sense.

ijpulidos commented 3 weeks ago

Actually, I noticed that we are not really exhaustively testing neither the partial charge assignation settings and variants, nor the handling of alchemical charge transformations. I think we need to add a minimal set of tests for these changes. Please suggest tests that we could add here, as possible.

ijpulidos commented 3 weeks ago

From the meeting it was agreed that we should port the host-guest system sanity check to this code base and set up a self-hosted runner on AWS (or similar) to run them, when possible.

ijpulidos commented 1 week ago

I just added the host-guest charge-changing transformation validation test. I created a test_sanity.py module for this, since I didn't know where to put it or even how to name it. I accept suggestions for this.

@hannahbaumann As discussed in our previous meeting, can you please take a look at the host-guest test and see if that seems reasonable? Thanks!. For now it's using the default settings for the protocol and it's running in ~4 hours in an A40 gpu (we could probably lower this time if we optimize the number of steps to get the convergence we need), the default runs 100 cycles of 12500 steps of 4ps.

I also marked the test as slow so it would only get run when --runslow is passed as an argument flag to pytest because we will be looking into running these tests on our self-hosted runners through GHA, once that's set up.

ijpulidos commented 1 day ago

As per the previous dev meeting, we decided that we also want to test the different options for assigning partial charges are working as expected in the code. This is a WIP, I'm writing the tests for this.