Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bugfix LJ Correction #425

Closed
wants to merge 5 commits into from
Closed

Conversation

fjclark
Copy link
Contributor

@fjclark fjclark commented Jan 13, 2023

This PR fixes the LJ correction and closes this issue.

I found two issues with the script:

1 . The LJ correction script consistently worked for me with Sire 2021.1.0, but started to fail for the complex legs of ABFE calculations for Sire 2022.1.0. This was due to overflow in the calculation of the exponential average of free energies. For energies more positive than - 420 kcal per mol at 298 K, exp(-E/kT) was just less than 1.7E308, causing no issues. However, for the complex, the LJ energy was greater, causing overflow of the float64s. The smaller LJ energy with the solvent leg system meant there were no issues. I'm not sure what update made for Sire 2022.1.0 caused the LJ energy of my complex leg system to increase to the extent that this issue surfaced. I've fixed this by subtracting the mean of the energies before exponential averaging and adding back on afterwards.

  1. Due to changes in molecule ordering made with Sire 2022.1, sometime the protein was selected as the solvent molecule. I've changed the logic to avoid this, but I don't think it's completely robust yet.

I've made the script compatible with recent changes to OpenMMMD.py. I've also modified runLambda() to return the correction, and resolveParameters() to allow return values to allow me to write a test for the correction. I'll open a PR for the test shortly.

chryswoods added a commit to OpenBioSim/sire that referenced this pull request Jan 17, 2023
)

and [michellab/Sire/pull/425](michellab/Sire#425).

Thanks to [@fjclark](https://github.com/fjclark) for submitting. Sorry for the lack
of authorship - I had to copy these files in manually across to the openbiosim repo.
@chryswoods
Copy link
Member

Hi @fjclark - Just to let you know that I have now merged this all into openbiosim/sire and it has been released as version 2023.1.1. I've also merged in your new unit test. This has been ported to openbiosim/sire_bigtests and will be run as part of the integration tests before each release.

If it is ok, I will close this pull request (and the other request) as these are now merged into the openbiosim code.

@fjclark
Copy link
Contributor Author

fjclark commented Jan 30, 2023

Hi @chryswoods, thanks very much. Of course.

@fjclark fjclark closed this Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LJ Correction Gives -inf
2 participants