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

Parallelization support via future.apply #25

Closed
wants to merge 4 commits into from
Closed

Conversation

ck37
Copy link
Contributor

@ck37 ck37 commented Apr 4, 2024

Hello,

Here is a small patch to support parallel processing for test_consequences_data_frame(). When it takes longer to calculate those stats, such as larger dataframes, more granular thresholds, and/or more models, this can have a nice speedup and I think the extra complexity is pretty minimal. For my current dataset this lets me go from 7 minutes to 45 seconds to run a dca(), so pretty helpful.

Happy to make any additional changes as preferred.

Cheers,
Chris

@ddsjoberg
Copy link
Owner

HI @ck37 ! Thanks for the post!

I am not sure when I'll get to this. I certainly see the benefit (quite obvious in this case). I need to consider deeply adding additional dependencies, particularly for functionality I have not used/not familiar with. (Well, I think I used it in grad school, but quickly forgot it!) I am sure you know, by accepting the PR, I am also signing up for maintaining the new code...forever.

@ddsjoberg
Copy link
Owner

Hi @ck37 , I am making a maintenance release of this package soon, which got me thinking again about your PR. Unfortunately, I don't think I want to take on the maintenance burden of the parallelization because I will find it difficult to continue to support into the future. I hope you understand.

@ddsjoberg ddsjoberg closed this Jul 22, 2024
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

Successfully merging this pull request may close these issues.

2 participants