Rotation+translation - coordinates appear changed

[GoogleCodeExporter]

Hi, 

working with dev/887ccee74, i've bumped into either a bug or counter intuitive 
behavior: as a random rotation matrix is applied, together with a translation, 
the coordinates in python memory appear changed but upon writing the change is 
not propagated.

Attached is a script with inputs to reproduce. Please load the 
  reference.pdb
  rotated.pdb
  translated.pdb

in VMD, to confirm that they have the same coordinates. 

Original issue reported on code.google.com by jan...@gmail.com on 25 May 2013 at 8:24

Attachments:

• IssueRotationTranslation.zip

[GoogleCodeExporter]

Okay, problem is contained to the PrimitivePDBWriter (gro files appear okay).

Original comment by jan...@gmail.com on 26 May 2013 at 1:19

[GoogleCodeExporter]

This issue was closed by revision f671817a3a2f.

Original comment by jan...@gmail.com on 26 May 2013 at 5:51

• Changed state: Fixed

[GoogleCodeExporter]

I kind of fixed the issue but it feels wrong: the PrimitivePDBWriter is now 
consistent with how AtomGroup.rotate() modifies the coordinates - but 
AtomGroup.rotate() itself is inconsistent (i claim) with the mode in which we 
pass the coordinates around. Let me explain: u.selectAtoms() creates a copy of 
atom coordinates, and this is sensible because one might:
  gr1 = u.selectAtoms("evil atoms")
  gr1.rotate(evil_matrix)
  gr2 = u.selectAtoms("good atoms")
  gr2.rotate(good_matrix)
The assumption then is that the coordinates of the underlying u (Universe) 
object remain unchanged, only the AtomGroups are rotated independently. That is 
not how atomgroup rotate() perceives the world... it will actually go and 
modify a slice of the universe coordinate array, which is (I claim) a bug 
heaven. 

Oli, I'm I getting it all terribly wrong?

Original comment by jan...@gmail.com on 27 May 2013 at 2:41

[GoogleCodeExporter]

Hi Jan,

> I kind of fixed the issue but it feels wrong: the PrimitivePDBWriter is now 
consistent with how AtomGroup.rotate() modifies the coordinates - but 
AtomGroup.rotate() itself is inconsistent (i claim) with the mode in which we 
pass the coordinates around. Let me explain: u.selectAtoms() creates a copy of 
atom coordinates, and this is sensible because one might:
>  gr1 = u.selectAtoms("evil atoms")
>  gr1.rotate(evil_matrix)
>  gr2 = u.selectAtoms("good atoms")
>  gr2.rotate(good_matrix)
>The assumption then is that the coordinates of the underlying u (Universe) 
object remain unchanged, only the AtomGroups are rotated independently. That is 
not how atomgroup rotate() perceives the world... it will actually go and 
modify a slice of the universe coordinate array, which is (I claim) a bug 
heaven. 

The idea was that any method that "acts" on an object actually changes the 
object itself. Thus

  ag.rotate(R)
  ag.translate(t)

do exactly what one expects: that part of the universe is affected. Thus you 
can use these methods for molecular modeling (e.g. rotate a dihedral, move a 
water molecule around, ...) and you can use multiple operations to build up the 
changes. If you were to only act on copies you could not do this.

Directly accessing the coordinates has a few more safeguards built in:

  ag.positions       # preferred
  ag.get_positions()
  ag.coordinates()   # deprecated

are copies. You can do whatever you like with these arrays and it won't affect 
the underlying Universe. This is a 'good thing'™ (I posit) because 
physically/chemically meaningful manipulations of the coordinates are generally 
restricted to the transformations available as methods and pretty much anything 
else one could do would mess up the universe.

Note, however, that ag.set_positions(a) (equivalent to ag.positions = a) 
actually changes the underlying coordinates, i.e. it does something like 
ts._pos[ag.indices(), :] = a. But you are fully responsible to change the 
coordinates in a manner that makes sense. 

So in summary: (1) I think you fixed the PrimitivePDBWriter in the correct 
manner. (2) The behaviour of AtomGroup.rotate() and friends is not "bug heaven" 
but an important feature ;-), at least in my opinion.

Oliver

Original comment by orbeckst on 11 Jun 2013 at 12:36