search

MDAnalysis / membrane-curvature

MDAnalysis tool to calculate membrane curvature.
https://membrane-curvature.readthedocs.io/
GNU General Public License v3.0
29 stars 6 forks source link

Trigger explicit warning when out of boundaries only once #95

Closed ojeda-e closed 1 year ago

ojeda-e commented 1 year ago

Description

As described in #86, the explicit warning messages introduced in #83 significantly affect performance. In this PR I introduced the WarnOnce class in surface.py, which provides an explicit message only once and after that triggers a generic message. It saves time when computing in membrane-protein systems, in particular. Tests were updated with regex that match both patterns.

Status

ojeda-e commented 1 year ago

@hmacdope maybe you can help me with a review here, please?

codecov[bot] commented 1 year ago

Codecov Report

Merging #95 (8b89f56) into main (ab0d652) will not change coverage. The diff coverage is 100.00%.

Additional details and impacted files
hmacdope commented 1 year ago

@ojeda-e I don't have write access so you can just approve and merge yourself.

ojeda-e commented 1 year ago

Thanks, @hmacdope!

Looks good to me @ojeda-e! Just out of curiosity what was the performance difference (roughly)?

I had a membrane-protein system with a 2500-frame trajectory and ~18900 atoms. Having explicit warning messages takes 57.38 seconds to iterate over the full number of frames, vs. 5.24 with the solution here implemented in WarnOnce. I understand it's just a particular case, but still, it is a significant difference. This solution saves quite a lot of time with the logger/print operation, which was too expensive with the explicit warning.

@ojeda-e I don't have write access so you can just approve and merge yourself.

Thanks for letting me know, I just added @MDAnalysis/gsoc-mentors as admin in the repo, it gave you write access right away.