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

Rmsd in smallmol docking #95

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Hackefleisch
Copy link
Member

This PR does two things:

  1. It fixes the RMSD accept/reject option in the TransformMover. The old version did not square the distances and was not automorphic. The new version simply uses the Rosetta automorphic rmsd function. Additionally, there is now an informative print to tell a user how many moves were rejected due to rmsd criteria.
  2. The RMSD criteria is copied 1 to 1 to the HighResMover and follows the same functionality.

Both changes are useful to limit docking exploration during execution towards the starting position and enforces local refinements.

Copy link
Member

@roccomoretti roccomoretti left a comment

Choose a reason for hiding this comment

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

You should add the test labels when this is ready to go.

I haven't done it for you, as it looks like it will fail the beautification tests. (Use tabs instead of spaces to indent Rosetta C++ code.)

ligand_index = i;
}
}
core::conformation::Residue original_ligand(pose.residue(ligand_index));
Copy link
Member

Choose a reason for hiding this comment

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

Prefer using a ResidueOP and .clone() to make a copy. I don't think it makes a difference here, but it does in other contexts where there's multiple derived types, it plays better with subtypes (the approach here would lose subtype info.)

for ( core::Size atomno = 1; atomno <= start.natoms(); ++atomno ) {
total_distance += start[atomno].distance(current[atomno]);
}
core::Real rmsd = core::scoring::automorphic_rmsd( *start.residue(), *current.residue(), /*superimpose=*/false );
Copy link
Member

Choose a reason for hiding this comment

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

Part of the purpose of UltraLightResidue is to not have to update the coordinates of the underlying conformation::Residue and take the overhead of that. As such, the residue() coordinates are set at initialization and not updated -- your change here isn't doing what you want it to (it's using stale coordinates).

Comment on lines +299 to +300
if ( residue.is_ligand() ) {
ligand_index = i;
Copy link
Member

Choose a reason for hiding this comment

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

There can theoretically be multiple ligands in the pose, only one of which (not necessarily the last) is the one which is being modified. Unfortunately, things are indirect enough that I don't think there's a good way of getting just the primary ligand from the info provided, so there likely isn't a need to change things. Just make sure to document the issue thoroughly. -- Though I might suggest breaking this out into a private function, which might make updating things later easier.

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