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

Add a global variable to set number of parallel threads #336

Merged
merged 10 commits into from
Jan 19, 2024
Merged

Conversation

LSchueler
Copy link
Member

@LSchueler LSchueler commented Dec 30, 2023

As pointed out by pavlovc2 in discussion #333, it would be helpful to set the number of parallel threads in GSTools directly and not only rely on environment variables.

I didn't have a lot of time to put this together, but in this first draft, you can set the number of threads with the global variable config.NUM_THREADS. At the moment, this only works with the Cython code and not with the Rust code. And also, for now, the default value of threads is 1.

TODO

  • also set the number of threads for the Rust code
  • find out how to set num_threads=None in the Cython code

@LSchueler LSchueler requested a review from MuellerSeb December 30, 2023 17:10
@LSchueler LSchueler self-assigned this Dec 30, 2023
@LSchueler LSchueler added enhancement New feature or request Performance Performance related stuff. labels Dec 30, 2023
@LSchueler
Copy link
Member Author

The Rust package GSTools-core is now ready for setting the number of parallel threads and I'm quite happy with the implementation.

For Cython, I used a bit of an ugly function to set defaults, without having to use Python variables.

I think the only thing left would be to update the changelog, if the review gets through ;-)

@LSchueler LSchueler marked this pull request as ready for review January 5, 2024 09:44
@MuellerSeb
Copy link
Member

See #337 for rtd issue.

@MuellerSeb
Copy link
Member

This looks promising. Was also winding my head around the fact, that prange uses None as default value.. this is idiotic to use a python type here. We should create an issue, that prange takes 0 or -1 to mimic the None behavior.
The function you used is ugly but I see that it's the best option at the moment.

Is it testable how many cores are used during a function call? This could be cool to check if the setting is actually working.

@LSchueler
Copy link
Member Author

I mean, we could get the numbers of cores in config.py and set it there and use num_threads=1 as the default values in the Cython and Rust functions. But it feels cleaner to get and set the default values in the Cython and Rust functions.

Regarding the testing, I think it's difficult to accurately test the number of threads used, if we don's simply want to check for the num_threads variable we just set.
I checked on my machine and the compute times decrease nearly linearly with the number of threads used (for num_threads < 13 at least). So, locally it works. But I'm not sure how many threads we can use in Github actions and if we really want to check the run times of different function calls.

@MuellerSeb
Copy link
Member

Created an issue in cython for prange: cython/cython#5952

@LSchueler LSchueler merged commit 90003fb into main Jan 19, 2024
25 of 26 checks passed
@MuellerSeb MuellerSeb added this to the 1.5.2 milestone Apr 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Performance Performance related stuff.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants