Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does Dihedral analysis use dimensions from separate universes? #215

Closed
orbeckst opened this issue Sep 8, 2022 · 3 comments
Closed

Does Dihedral analysis use dimensions from separate universes? #215

orbeckst opened this issue Sep 8, 2022 · 3 comments

Comments

@orbeckst
Copy link
Member

orbeckst commented Sep 8, 2022

It looks as if DihedralAnalysis takes a short cut and only uses the dimensions of the first ensemble member:

angle = calc_dihedrals(cord1, cord2, cord3, cord4,
box=self.g1[key_list[0]].dimensions)

This can lead to wrong results. Instead, the calculations need to be done for each universe separately.

@orbeckst
Copy link
Member Author

orbeckst commented Sep 8, 2022

@ALescoulie can you please explain what's happening in DihedralAnalysis — did I misunderstand how it works?

@orbeckst orbeckst added question and removed bug labels Sep 8, 2022
@orbeckst orbeckst changed the title Dihedral analysis should use dimensions from separate universes Does Dihedral analysis use dimensions from separate universes? Sep 8, 2022
@ALescoulie
Copy link
Contributor

@orbeckst In _single_frame it's looking at stuff from a single universe, I just split in into 4 EnsembleAtomGroups. At the beginning _reorg_groups takes the given dihedral selections and makes 4 EnsembleAtomGroups each just containing the one atoms of each dihedral in the inputted dihedrals. This done for efficiency (I think I found that it was slower in my report) since we could do the dihedral calculations together with only one call to Cython. One of the functions called in __init__ ensures the passed in list is all from the same Ensemble. I don't think this is an issue.

@orbeckst
Copy link
Member Author

orbeckst commented Sep 8, 2022

Thank you for the explanation. I didn't understand how the current universe is selected. This all makes more sense now.

(Incidentally, you had actually fixed the box in #198 ).

@orbeckst orbeckst closed this as not planned Won't fix, can't repro, duplicate, stale Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants