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

fp.to_dataframe(return_atoms=True,drop_empty=True) still slow #112

Closed
ReneHamburger1993 opened this issue Apr 3, 2023 · 1 comment · Fixed by #113
Closed

fp.to_dataframe(return_atoms=True,drop_empty=True) still slow #112

ReneHamburger1993 opened this issue Apr 3, 2023 · 1 comment · Fixed by #113

Comments

@ReneHamburger1993
Copy link
Contributor

Hi,

I have 25k frames that I analyzed and it took ~5min to get the full atomid DataFrame with my selections.

I traced it down to the grouping including the "empty" columns.

If we remove them before grouping it gets much faster.

1000 frames: 10s -> ~2s
25k frames: ~300s -> 60s

Though its a factor of 5 faster now.
I will open a PR with my optimization.

Kind regards
René
ReneHamburger1993 added a commit to ReneHamburger1993/ProLIF that referenced this issue Apr 3, 2023
@ReneHamburger1993
Update utils.py
d9cf65a 
PR to fix [chemosim-lab#112
ReneHamburger1993 added a commit to ReneHamburger1993/ProLIF that referenced this issue Apr 3, 2023
@ReneHamburger1993
Fix chemosim-lab#112
54b6535 
drop cols containing None before grouping. Speeds the function up in my case: ~x5
when called as `fp.to_dataframe(return_atoms=True,drop_empty=True)`
@ReneHamburger1993 ReneHamburger1993 mentioned this issue Apr 3, 2023
Merged
@cbouy
Copy link
Member

cbouy commented Apr 5, 2023

Hi René and thanks a lot for reporting this and contributing, I'll try to have a look at your PR this week.

Best,
Cédric

@cbouy cbouy closed this as completed in #113 Apr 5, 2023
cbouy added a commit that referenced this issue Apr 5, 2023
@ReneHamburger1993 @cbouy
Rene hamburger1993 patch 1 (#113)
e2497c1 
* Fix #112

drop cols containing None before grouping. Speeds the function up: ~x5
when called as `fp.to_dataframe(return_atoms=True, drop_empty=True)`

---------

Co-authored-by: Cédric Bouysset <bouysset.cedric@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rene hamburger1993 patch 1 ReneHamburger1993/ProLIF
2 participants
@cbouy @ReneHamburger1993