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

AtomGroup.rotate is slow #1707

Closed
mimischi opened this issue Nov 21, 2017 · 4 comments
Closed

AtomGroup.rotate is slow #1707

mimischi opened this issue Nov 21, 2017 · 4 comments

Comments

@mimischi
Copy link
Contributor

Expected behaviour

Using universe.atoms.rotate(rotation_matrix) should be as fast as np.dot(universe.atoms.postions, rotation_matrix).

Actual behaviour

The rotate method is almost 7x slower than np.dot. In the test system it's almost unnoticeable (70μs vs 490μs), but in my production system it's a difference of ~30 seconds (with numpy) and ~2m30s (with MDAnalysis).

Code to reproduce the behaviour

See this notebook.

In my production system I first translate and then rotate my system. To isolate the exact problem, I've used all variations: rotate only, translate only and rotate+translate. Also I've done the tests on both single frames and while iterating over all frames.

Currently version of MDAnalysis:

0.16.2

@kain88-de kain88-de mentioned this issue Nov 21, 2017
2 tasks
@richardjgowers
Copy link
Member

One difference between the numpy and mda versions of things is MDA ensures that the positions it works on are all unique. It would be interesting to see if commenting out the uniqueness check in MDA improves performance a lot for your large system

@kain88-de
Copy link
Member

what position uniqueness check?

@richardjgowers
Copy link
Member

@kain88-de unique atoms more than positions I guess https://github.com/MDAnalysis/mdanalysis/blob/develop/package/MDAnalysis/core/groups.py#L827

this line looks needlessly slow too, no reason to go via atoms.unique, could just be x = self.universe....

@kain88-de
Copy link
Member

I was wondering about the use of unique here anyway.

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

No branches or pull requests

3 participants