search

mosdef-hub / mbuild

A hierarchical, component based molecule builder
https://mbuild.mosdef.org
Other
171 stars 80 forks source link

Combine rotation commands #159

Closed ctk3b closed 8 years ago

ctk3b commented 8 years ago

It might be more elegant to have just two rotation commands:

rotate(compound, thetas) and rotate_around_axis(compound, thetas)

where thetas is an array of 3 angles.

sallai commented 8 years ago

I had thought about that, too, but I decided not to. The problem is that rotations around different axes are not commutable operations. It is a very rare case that the user first wants to rotate around the x axis, then around the y, and finally around the z.

On Tue, Dec 8, 2015 at 11:01 AM Christoph Klein notifications@github.com wrote:

It might be more elegant to have just two rotation commands:

rotate(compound, thetas) and rotate_around_axis(compound, thetas)

where thetas is an array of 3 angles.

— Reply to this email directly or view it on GitHub https://github.com/iModels/mbuild/issues/159.

ctk3b commented 8 years ago

Hmm yeah didn't quite think that through.

chrisiacovella commented 8 years ago

You can still use the rotate command to just rotate about one axis at a time if that were for some reason useful.

sallai commented 8 years ago

That is true. But still, I think that having the name of the method explicitly describe what axis we want to rotate around would be a bit more user friendly:

rotate_around_z(compound, np.pi/4) rotate(compoud, [0, 0, np.pi/4])

On Wed, Dec 9, 2015 at 9:41 AM chrisiacovella notifications@github.com wrote:

You can still use the rotate command to just rotate about one axis at a time if that were for some reason useful.

— Reply to this email directly or view it on GitHub https://github.com/iModels/mbuild/issues/159#issuecomment-163296892.

ctk3b commented 8 years ago

We may want to come up with a better name than revolve_around_x for rotating in place though since that sounds like it does the same thing. Any ideas?

I might even lean towards swapping the names or perhaps "spin"?

chrisiacovella commented 8 years ago

A spin operator sounds useful, since we often use that for initializing monolayers/bilayers (e.g., spin the molecule around it's long axis to randomize orientations). So to differentiate, spin could be a 2d operation, rotate is the 3d rotation matrix.

sallai commented 8 years ago

Yes, I like spin.

On Wed, Dec 9, 2015 at 1:01 PM chrisiacovella notifications@github.com wrote:

A spin operator sounds useful, since we often use that for initializing monolayers/bilayers (e.g., spin the molecule around it's long axis to randomize orientations). I think that is better than calling it rotate, since we can say rotate is the 3d rotation matrix.

— Reply to this email directly or view it on GitHub https://github.com/iModels/mbuild/issues/159#issuecomment-163358182.

chrisiacovella commented 8 years ago

This might be useful for the forcefield fitting stuff, too. For instance, if we want to calculate the dihedral, we'll spin around a specific bond.