Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jadeshi
Copy link

@jadeshi jadeshi commented May 16, 2013

... 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.

…ing 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.
@kyleabeauchamp
Copy link
Contributor

Nice find!

@rmcgibbo
Copy link
Contributor

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?

@jadeshi
Copy link
Author

jadeshi commented May 16, 2013

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.

@rmcgibbo
Copy link
Contributor

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

@jadeshi
Copy link
Author

jadeshi commented May 16, 2013

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?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants