michellab / Sire

Sire Molecular Simulations Framework
http://siremol.org
GNU General Public License v3.0
95 stars 26 forks source link

Boresch Restraints for Absolute Binding Free Energy Calculations #392

Closed fjclark closed 1 year ago

fjclark commented 2 years ago

Hello,

I've implemented Boresch restraints for ABFE calculations, trying to follow the implementation of the distance restraints as closely as possible. I've also implemented the analytical correction (Equation 32 of the paper) and semi-analytical correction for releasing these restraints to the standard state. The code for the latter is very closely based on restraints.py in Yank - I have acknowledged this, and my understanding is that this is fine due to the compatibility of the MIT and GNU GPL licenses.

This allows ABFE calculations with Boresch restraints to be run in three stages:

- Vanishing and discharging stages are completed as with multiple distance restraints.

I originally defined the force constants as F = k\*x, as in the original paper, but I have changed this to F = 2\*k\*x to be as consistent as possible with the multiple distance restraints. The corrections can be calculated by feeding any config file containing a Boresch restraints dictionary to the correction apps, for example:

boresch_analytical_correction -C ../input/sim.cfg



I have begun work on tests, but I am unsure of the best approach. I was planning to copy most of the functionality over from OpenMMMD.py to set up a system with and without restraints, and test for the correct difference in energies. Do you have any recommendations?

Thanks very much,
Finlay
chryswoods commented 2 years ago

Thanks @fjclark. It looks good. We will try to find a way to get the GitHub Actions to run so that we can test it, but I think it should be fine.

Yes, you are right that you can include MIT (or any properly open source license, like Apache, BSD etc) code in a GPL package. The licenses are compatible. We just have to retain copyright notices and make sure that, if code is inspired by another code, that attribution and the original license and copyright is recorded somewhere. As part of the feat_web work, I am creating a new website that provides extra attribution for such code, e.g. see here.

Speaking of feat_web, your pull request and work should not be impacted by it. I'll do the work to pull your change into feat_web and also sort out the new way that we will be handling the tests :-)

fjclark commented 2 years ago

Great, thanks very much @chryswoods, and thanks for the comments.

lohedges commented 2 years ago

Apologies for the delay with this @fjclark. I'll take a look on a local copy to see if I can resolve the conflicts and apply any 2023 related updates. I'm a bit confused why this would revert certain changes, i.e. those in the CHANGELOG, but I guess this is to do with the point at which your fork was made.

fjclark commented 2 years ago

No problem at all @lohedges. Brilliant, thank you. Yes, I don't remember any conflicts at the time I originally made the PR.

lohedges commented 1 year ago

Hi @fjclark. I've done the work to update your scripts for the 2023 API and have also dealt with the various conflicts. This has been pushed to the feature-boresch branch. Could you take a look at this and let me know when you are happy?

fjclark commented 1 year ago

Hi @lohedges, thanks very much for this - looks good. A couple of small comments:

Otherwise I'm happy.

Cheers.

lohedges commented 1 year ago

No problem, I'll add these tomorrow. I must have missed them when updating. (I had to manually edit certain files because of the conflicts.)

fjclark commented 1 year ago

Perfect, thanks.

lohedges commented 1 year ago

Closed as incorporated via b9ef695.