Change DFTK thread count to a runtime setting #1028
Conversation
src/common/threading.jl
Outdated
| """ | ||
| function disable_threading() | ||
| setup_threading(;n_fft=1, n_blas=1, n_DFTK=1) | ||
| end | ||
|
|
||
| # TODO: is a single write to an Int64 atomic? | ||
| DFTK_threads = 0 |
There was a problem hiding this comment.
Should be const for sure.
But also this is global state. Do we really want this?
There was a problem hiding this comment.
Apparently according to Valentin the way to go is https://github.com/vchuravy/ScopedValues.jl, which will become part of base in 1.11. Can you look into this ?
There was a problem hiding this comment.
Funny how that's actually inspired from a Java API. 😄
I'm not sure that
with(DFTK_THREADS => 1) do
# All computations here
endis very user friendly however, especially since it's inconsistent with BLAS and FFTW threads.
It's not REPL-friendly either.
There was a problem hiding this comment.
Making it const is a bad idea, the point is that it's actually mutable (and will be mutated by set_DFTK_threads!):
WARNING: redefinition of constant DFTK.DFTK_threads. This may fail, cause incorrect answers, or produce other errors.
There was a problem hiding this comment.
const does not mean that it's immutable if you make it a Ref. It most importantly it ensures the type does not change.
also with(DFTK_THREADS => 1) do can be wrapped.
There was a problem hiding this comment.
I talked to Valentin again, and the ideal thing to do would be to split work based on statically known block sizes (wherever we call parallel_loop_over_range at least), and let the Julia scheduler handle this. However I don't want to do this now.
|
Ok, let's do it like this for now. We should really revisit our HPC setup and scheduling (like have some proper structured way of configuring this). |
Fixes #1012 by changing the number of DFTK threads to a runtime setting.
Do we care about potential races when calling
set_DFTK_threads!()in parallel? I suppose not. I don't want to make it an atomic.