Skip to content
This repository was 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
jadeshi wants to merge 1 commit into
msmbuilder:masterfrom
jadeshi:patch-1
Open

Fixed a bug in msmbuilder/Extras/parallel_assign/lib/remote.py regarding...#195
jadeshi wants to merge 1 commit into
msmbuilder:masterfrom
jadeshi:patch-1

Conversation

@jadeshi

@jadeshi jadeshi commented May 16, 2013

Copy link
Copy Markdown

... 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
Copy Markdown
Contributor

Nice find!

@rmcgibbo

Copy link
Copy Markdown
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

jadeshi commented May 16, 2013

Copy link
Copy Markdown
Author

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
Copy Markdown
Contributor

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

@jadeshi

jadeshi commented May 16, 2013

Copy link
Copy Markdown
Author

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