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.

Sign up for GitHub

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

Jump to bottom

Improve DCD reader typing and performance. #3882

Closed
hmacdope opened this issue Oct 26, 2022 · 3 comments · Fixed by #3888
Closed

Improve DCD reader typing and performance. #3882

hmacdope opened this issue Oct 26, 2022 · 3 comments · Fixed by #3888
Assignees
@hmacdope
Labels
Component-Readers CZI-performance performance track of CZIEOSS4 grant Format-DCD

Comments

@hmacdope
Copy link
Member

The Cython DCD reader could do with a little bit of an update. There was a large regression in its performance compared to the other readers as measured by the ASV benchmarks around 6ae0eb12 See attached screen-cap of the ASV benchmarks.

There is room for improvement in the typing used in the cython. Cython can be very particular about the use of types wherever possible. Inspection of the annotated html indicates there is room for improvement that will hopefully help with performance.

Screenshot from 2022-10-26 15-35-43
@hmacdope hmacdope added Component-Readers Format-DCD CZI-performance performance track of CZIEOSS4 grant labels Oct 26, 2022
@hmacdope hmacdope self-assigned this Oct 26, 2022
@Luthaf
Copy link
Contributor

Luthaf commented Oct 26, 2022

I found that the performance of MDA DCD reader depends a lot on the OS/filesystem combo (cf https://github.com/chemfiles/benchmarks/). I don't understand how that would happen, but might be interesting to investigate.

@hmacdope
Copy link
Member Author

@Luthaf from reading the benchmarks it looks like a huge performance loss on your ubuntu benchmark? Interesting I will keep in mind.

@IAlibay
Copy link
Member

IAlibay commented Oct 26, 2022

I suspect the M1's SoC architecture might have quite some involvement here?

This was referenced Oct 27, 2022
Merged
Closed
@hmacdope hmacdope moved this to In Progress in CZI Performance track Nov 3, 2022
orbeckst pushed a commit that referenced this issue Nov 29, 2022
@hmacdope
Improve libdcd cython (#3888)
d63cd6d 
* Fixes #3882 and #3923
* Improves the cython in the DCD reader to be more compiled.
* add annotate_cython setup.cfg option
* add types and fix is_periodic
* make int cast explicit
* change whence dict
* move DCD reader to no copy API and update tests accordingly
* add temporary buffer to DCD reader
* add module level directives
* pxdify libdcd: add new lib/formats/libdcd.pxd
* Add deprecation warning for `timestep` copying in DCDReader (Issue #3889)
* add DCD to public libmdanalysis API
* update tests
* update CHANGELOG
Repository owner moved this from In Progress to Done in CZI Performance track Nov 29, 2022
@hmacdope hmacdope mentioned this issue Feb 26, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hmacdope hmacdope

Labels
Component-Readers CZI-performance performance track of CZIEOSS4 grant Format-DCD
Projects
CZI Performance track
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Improve libdcd cython hmacdope/mdanalysis
3 participants
@Luthaf @IAlibay @hmacdope