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

Convert Restraints to CV-Based #1059

Merged
merged 3 commits into from
Aug 3, 2018
Merged

Convert Restraints to CV-Based #1059

merged 3 commits into from
Aug 3, 2018

Conversation

Lnaden
Copy link
Contributor

@Lnaden Lnaden commented Aug 2, 2018

This PR converts ALL of the restraints to CustomCVForce objects.

This feature now requires OpenMMTools 0.16.0, which is still in development, but the local tests pass. The OpenMMTools PR/branch which I am using is here:

choderalab/openmmtools#363

This changes many things so could use a review. Many of the changes here are reflections of the linked OpenMMTools issues

Copy link
Contributor

@andrrizzi andrrizzi left a comment

Choose a reason for hiding this comment

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

Looks good! Spotted a couple of typos to fix before merging.

@@ -290,7 +290,8 @@ def test_partial_parametrization():
particles, _ = force.getBondParameters(0)
assert particles == tuple(restraint.restrained_receptor_atoms + restraint.restrained_ligand_atoms)
# RMSD restraint.
elif OpenMM73.dev_validate and isinstance(force, openmm.CustomCVForce):
elif OpenMM73.dev_validate and isinstance(force, openmm.CustomCVForce) and \
Copy link
Contributor

Choose a reason for hiding this comment

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

If remember correctly, the OpenMM73.dev_validate we added some time ago is unnecessary now since the CV force is openmm>=7.2 and we've pinned it in the conda recipe, correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RMSD Force is still a 7.3 thing, but I need to fix that to be more generic

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah! I see, I forgot about the RMSD restraint.

self.restrained_ligand_atoms[0],
self.restrained_ligand_atoms[1],
[])
force_components.append(['TorsionB', torsion_b_force])
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you forgot to change TorsionB to boresch_torsion_b.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I did, that might explain the NaN's

self.restrained_ligand_atoms[1],
self.restrained_ligand_atoms[2],
[])
force_components.append(['TorsionC', torsion_c_force])
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here.

@Lnaden Lnaden merged commit bdfc09d into multirestraint Aug 3, 2018
@Lnaden Lnaden deleted the restraintcv branch August 3, 2018 14:38
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.

2 participants