Skip to content

Modified MC rotation moves #331

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

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

Modified MC rotation moves #331

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
99ed70f
modified MCRotationMove
kamran-haider Feb 8, 2018
6497bad
better name for indices in modified rotate_positions
kamran-haider Feb 8, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions openmmtools/mcmc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1782,13 +1782,15 @@ def __init__(self, **kwargs):
super(MCRotationMove, self).__init__(**kwargs)

@classmethod
def rotate_positions(cls, positions):
def rotate_positions(cls, positions, rotation_center_indices=None):
"""Return the positions after applying a random rotation to them.

Parameters
----------
positions : nx3 numpy.ndarray simtk.unit.Quantity
The positions to rotate.
rotation_center_indices : list
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a , optional here? Also, it would be fantastic to document that the center of rotation is chosen to be the center of geometry of this set of atoms, and that the default behavior uses all the atoms in positions.

Copy link
Author

Choose a reason for hiding this comment

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

No worries at all and thanks so much for the comments. I agree and will get back with the unit test and updated docstring.

Indices of a subset of atoms to center the rotation around, None by default.

Returns
-------
Expand All @@ -1799,8 +1801,16 @@ def rotate_positions(cls, positions):
positions_unit = positions.unit
x_initial = positions / positions_unit

# Compute center of geometry of atoms to rotate.
x_initial_mean = x_initial.mean(0)
# Define coordinates for the center of rotation.
x_rot_centers = x_initial

# Update coordinates for the center of rotation from the subset defined by indices
if rotation_center_indices is not None:
if len(rotation_center_indices) < 1 or len(rotation_center_indices) > x_initial.shape[0]:
raise IndexError("Length of rotation center indices must be >= % d and <= %d" % (1, x_initial.shape[0]))
x_rot_centers = positions[rotation_center_indices, :]

x_initial_mean = x_rot_centers.mean(0)

# Generate a random rotation matrix.
rotation_matrix = cls.generate_random_rotation_matrix()
Expand Down