djay0529 / mdanalysis

Automatically exported from code.google.com/p/mdanalysis
0 stars 0 forks source link

Add forces and/or velocities to AtomGroup when TimeStep has no forces/velocities defined #213

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Load a CHARMM DCD trajectory (u = MDAnalysis.Universe('a.psf', 'a.dcd')
2. a = u.selectAtoms('all')
3. a.set_forces(0.)

What is the expected output? What do you see instead?
Ideally, no output, simply that forces/velocities have been set. Instead, I 
receive the following error:

NoDataError: Timestep does not contain forces

What version of the product are you using? On what operating system?
MDAnalysis 0.8.2dev

Please provide any additional information below.

Ideally, one could set forces/velocities and create a new trajectory or work 
with the current system instead of having multiple systems available.

Original issue reported on code.google.com by tcthepoe...@gmail.com on 10 Feb 2015 at 5:33

GoogleCodeExporter commented 9 years ago

Original comment by richardjgowers on 10 Feb 2015 at 9:23

GoogleCodeExporter commented 9 years ago
It would be nice to change the Position/Velocity/Force access in Timesteps to 
use managed properties.

so something like

@property
def forces(self):
    try:
        return self._forces
    except AttributeError

@forces.setter
def forces(self, new):
    try:
        self._forces[:] = new
    except AttributeError:
        self._forces = np.zeros(len(self))
        self._forces[:] = new

Then all access to this information could be done via these properties rather 
than using the "private" attributes, so ts._pos could be changed to 
ts.positions in core.AtomGroup.

Original comment by richardjgowers on 10 Feb 2015 at 9:44

GoogleCodeExporter commented 9 years ago
Using managed attributes for Timestep.positions, velocities, forces in the way 
that you outline sounds like a solution to the problem.

Maybe not for 0.9.0 yet but 0.9.1?

Original comment by orbeckst on 10 Feb 2015 at 1:59

GoogleCodeExporter commented 9 years ago
Hi,

Just a note that something along these lines has already been done for TRRs, 
which don't always carry position/force/velocity information (Issue 162).

A completely format-independent approach would be a good addition, IMO.

Original comment by manuel.n...@gmail.com on 10 Feb 2015 at 2:36

GoogleCodeExporter commented 9 years ago
Yeah I did think of the solution for TRR.  That's an odd one because in that 
"_pos" itself is the managed property.

If we put the managed properties into base.Timestep it would probably tidy up 
TRR.Timestep a lot.

Original comment by richardjgowers on 10 Feb 2015 at 3:21

GoogleCodeExporter commented 9 years ago
I think my rationale there, since I was only dealing with the TRR part, was to 
prevent stuff that relied on _pos to keep from breaking in case it'd be absent.

But I totally agree with putting it into the base class, and making the higher 
level positions the managed property.

Original comment by manuel.n...@gmail.com on 10 Feb 2015 at 3:38

GoogleCodeExporter commented 9 years ago
So this will likely require changes to core.AtomGroup and most coordinate 
modules, so I think 0.9.1 too.

Original comment by richardjgowers on 11 Feb 2015 at 1:19

GoogleCodeExporter commented 9 years ago

Original comment by richardjgowers on 20 Feb 2015 at 11:14

GoogleCodeExporter commented 9 years ago

Original comment by richardjgowers on 20 Feb 2015 at 11:37

GoogleCodeExporter commented 9 years ago
After fixing Issue 224 it came to my attention that the link between _pos and 
_x, _y, and _z is quite fragile. It is very easy to do something like:

timestep._pos = somearray

and in the process lose the _x, _y, _z views onto _pos.
Likewise such an assignment to any of the dimensions shortcuts also breaks the 
view.

This would work if assignment is done to _pos[:], but it isn't obvious that it 
should be so.

The solution also passes through having managed properties, and in fact TRRs 
now accept direct assignment to these arrays without ill effects. I'm leaving 
this here so that when we fix this issue we don't forget about the coordinate 
shortcut getter/setter decorators.

Original comment by manuel.n...@gmail.com on 17 Mar 2015 at 5:18

GoogleCodeExporter commented 9 years ago
I've got a solution to this in progress, should make all access to positions 
via getter/setters so this kind of thing can't happen!

I'll probably rope you in to check the TRR case, that seems especially tricky.

Original comment by richardjgowers on 17 Mar 2015 at 5:56