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.

Sign up for GitHub

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

Jump to bottom

Fix issue #262 #263

Merged
merged 14 commits into from
Dec 3, 2024
Merged

Fix issue #262 #263

merged 14 commits into from
Dec 3, 2024

Conversation

lohedges
Copy link
Contributor

@lohedges lohedges commented Nov 29, 2024 •
edited
Loading

This PR closes #262 by fixing several minor issues with the Sire-EMLE implementation:

  1. The OpenMM platform can changing during minimisation, which means that the EMLE Torch module needs to move device too. Annoyingly calling the .to() method from C++ doesn't appear to call the overloaded .to() method in Python, so I've needed to reload the module and move it. (This doesn't seem to be slow and is only done, at most, once during minimisation.)
  2. I've updated the docs to show how to minimise before running production QM/MM.
  3. I've fixed the accumulation of non-equilibrium work when interpolating from QM to MM. The original equation assumed MM to QM, but this isn't always the case. The total work is now symmetric if you run the same system forwards or backwards.

Also closes #248. I've now pulled in the changes from #258, so that will be merged alongside this.

Also closes #259. I've pulled in my thread safety fix for the OpenMM minimiser from my feature_repex branch.

Also closes #261 by fixing the assignment/detection of stereochemistry in conversion to/from RDKit. This fix also closes OpenBioSim/biosimspace#370.

  • I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): [y]
  • I confirm that I have added a changelog entry to the changelog (we will add a link to this PR as part of the review): [y]
  • I confirm that I have permission to release this code under the GPL3 license: [y]

fjclark and others added 5 commits November 1, 2024 13:34
@fjclark
Make Boresch restraints correction consistent with F=2kx
be8aa3a 
Previously, the correction was implemented assuming F=kx. This lead
to erroneously favourable absolute free energies of binding by
1.23 kcal mol-1.
@fjclark
Update docs to make clear that F = 2kx for restraints
63e76ea 
Update the restraint function docstrings, restraints tutorial,
and changelog.
@lohedges
Workaround for device change during minimisation. [ref #262]
1bc4e46
@lohedges
Add minimisation step to QM/MM tutorials. [ref #262]
3e866de
@lohedges
Fix QM-to-MM non-equilibrium work. [ref #262]
c064358
@lohedges lohedges added the bug Something isn't working label Nov 29, 2024
@lohedges lohedges requested a review from mb2055 November 29, 2024 11:52
@lohedges lohedges self-assigned this Nov 29, 2024
@lohedges lohedges added the documentation Improvements or additions to documentation label Dec 3, 2024
mb2055
mb2055 approved these changes Dec 3, 2024
View reviewed changes
Copy link
Contributor

@mb2055 mb2055 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good 👍

@lohedges lohedges merged commit dec3ce8 into devel Dec 3, 2024
5 checks passed
@lohedges lohedges deleted the fix_262 branch December 3, 2024 17:19
lohedges added a commit that referenced this pull request Dec 4, 2024
@lohedges
Backport fixes from PR #263. [ci skip]
6dc3126
@lohedges lohedges mentioned this pull request Dec 4, 2024
Merged
lohedges added a commit that referenced this pull request Dec 4, 2024
@lohedges
Merge pull request #264 from OpenBioSim/backport_263
1688496 
Backport fixes from PR #263
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mb2055 mb2055 mb2055 approved these changes

Assignees

@lohedges lohedges

Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Milestone
No milestone
3 participants
@lohedges @mb2055 @fjclark