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

convert_to('PARMED') super slow #3028

Closed
fhh2626 opened this issue Nov 17, 2020 · 2 comments · Fixed by #3029
Closed

convert_to('PARMED') super slow #3028

fhh2626 opened this issue Nov 17, 2020 · 2 comments · Fixed by #3029

Comments

@fhh2626
Copy link

fhh2626 commented Nov 17, 2020

Expected behavior

The call of the method should be finished within a reasonable time scale

Actual behavior

For a 10k-atom system, this function call takes 2-3 min, and for a 60k-atom system, this takes > 15 min.

Code to reproduce the behavior

import MDAnalysis as mda
import parmed as pmd
prm = pmd.load_file('whatever.parm7', 'whatever.rst7')
u = mda.Universe(prm)
prot = u.select_atoms('all')
prm_u = prot.convert_to('PARMED')

....

Current version of MDAnalysis

  • Which version are you using? (run python -c "import MDAnalysis as mda; print(mda.__version__)")
    the latest conda version of MDAnalysis and parmed
  • Which version of Python (python -V)?
    Python 3.7.8
  • Which operating system?
    I tried both Windows 10 and Linux Mint 19
HanatoK added a commit to HanatoK/mdanalysis that referenced this issue Nov 17, 2020
The origin code used index() of list to lookup the atom indices, which
is nearly O(N^2) when iterating all atoms. This commit converts the list
to a dictionary mapping the atom objects to indices, and hence improves
the overall performance.
@orbeckst orbeckst added this to the 2.0 milestone Nov 18, 2020
IAlibay pushed a commit that referenced this issue Nov 19, 2020
Fixes #3028 

## Work done in this PR
* Improves the performance of the ParmEd converter by using a dictionary lookup for the atomgroup to universe index mapping.
@IAlibay
Copy link
Member

IAlibay commented Nov 19, 2020

Just re-opening to confirm that sufficient work has been done here to address the issue.

@fhh2626 would you be able to confirm that the latest fix (#3029) alleviates your issue? (this would involve installing the developer version of MDAnalysis).

@IAlibay IAlibay reopened this Nov 19, 2020
@fhh2626
Copy link
Author

fhh2626 commented Nov 19, 2020

Yes, thanks for your nice work!

@fhh2626 fhh2626 closed this as completed Nov 19, 2020
lilyminium pushed a commit to lilyminium/mdanalysis that referenced this issue Dec 8, 2020
…DAnalysis#3029)

Fixes MDAnalysis#3028

* Improves the performance of the ParmEd converter by using a dictionary lookup for the atomgroup to universe index mapping.
lilyminium pushed a commit to lilyminium/mdanalysis that referenced this issue Dec 8, 2020
…DAnalysis#3029)

Fixes MDAnalysis#3028

* Improves the performance of the ParmEd converter by using a dictionary lookup for the atomgroup to universe index mapping.
lilyminium pushed a commit to lilyminium/mdanalysis that referenced this issue Dec 9, 2020
…DAnalysis#3029)

Fixes MDAnalysis#3028

* Improves the performance of the ParmEd converter by using a dictionary lookup for the atomgroup to universe index mapping.
lilyminium pushed a commit to lilyminium/mdanalysis that referenced this issue Dec 14, 2020
…DAnalysis#3029)

Fixes MDAnalysis#3028

* Improves the performance of the ParmEd converter by using a dictionary lookup for the atomgroup to universe index mapping.
PicoCentauri pushed a commit to PicoCentauri/mdanalysis that referenced this issue Mar 30, 2021
…DAnalysis#3029)

Fixes MDAnalysis#3028 

## Work done in this PR
* Improves the performance of the ParmEd converter by using a dictionary lookup for the atomgroup to universe index mapping.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants