msmbuilder-legacy icon indicating copy to clipboard operation
msmbuilder-legacy copied to clipboard

Fixed a bug in msmbuilder/Extras/parallel_assign/lib/remote.py regarding...

Open jadeshi opened this issue 12 years ago • 5 comments

... application of AtomIndices.dat

Generators were mistakenly screened with AtomIndices.dat, when all atoms from generators should be used. This fix sets the atom indices attribute to the 'metric' object to None when computing the generators and then restores it afterwards. I've tried this on the ala2 system and it produced correct results.

jadeshi avatar May 16 '13 21:05 jadeshi

Nice find!

kyleabeauchamp avatar May 16 '13 21:05 kyleabeauchamp

Perhaps an easier way to fix it would be to use the AtomIndices flag to Trajectory.load_trajectory_file (in the traj, not the generators), and then not use any AtomIndices in the metric object?

rmcgibbo avatar May 16 '13 22:05 rmcgibbo

I see, that's definitely cleaner. Is there somewhere else the code that requires the metric object to have an 'atomindices' attribute? If not it seems like it could be made to by default not have one.

jadeshi avatar May 16 '13 22:05 jadeshi

Yeah, it's optional. When the RMSD object doesn't get an atomindices, it just uses all of them.

rmcgibbo avatar May 16 '13 22:05 rmcgibbo

It seems like it may be a bit awkward to change though, since in the current code the indices are specified with the metric (e.g. rmsd -a AtomIndices.dat) and assigned to the metric, so would we have to change the parsing scheme too?

jadeshi avatar May 16 '13 22:05 jadeshi