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

Distributed: Set number of BLAS threads if LinearAlgebra is loaded, otherwise do nothing #28150

Closed
wants to merge 1 commit into from

Conversation

KristofferC
Copy link
Member

Breaks a dependency from Distributed -> LinearAlgebra.

@StefanKarpinski
Copy link
Member

Can we use package_callbacks to deal with the other load order?

@andreasnoack
Copy link
Member

Can we use package_callbacks to deal with the other load order?

I more general solution might be useful here since e.g. FFTW might also want to use this.

@KristofferC
Copy link
Member Author

I more general solution might be useful here since e.g. FFTW might also want to use this.

Could you propose something? A vector in Base with callbacks that packages put there to run with n threads?

const thread_callbacks = []
function set_library_threads(n::Int)
    for f in thread_callbacks
        f(n)
    end
end

@andreasnoack
Copy link
Member

Could you propose something?

Unfortunately, I'm not sure how this could be done. I just realized that this might be relevant beyond LinearAlgebra. Maybe these packages should have some Requires like code that sets the number of threads if Distributed has been loaded.

@KristofferC
Copy link
Member Author

KristofferC commented Jul 18, 2018

Why does loading "Distributed" do "magic things" to other libraries (BLAS). Why can't it be just a doc thing, "if you do distributed computing, make sure your libraries only use one core" or something.

@andreasnoack
Copy link
Member

Why can't it be just a doc thing,

It could and it would indeed simplify things. It's not unreasonable to ask people to actively set the number of threads when using multiple processes. Often you'd probably want a combination of threads and processes anyway. So far we have assumed that people wanted to disable threading when using multiple processes but we could stop assuming that. As you've pointed out, the dependency structure required by the current assumption is also a bit unfortunate.

@kshyatt kshyatt added parallelism Parallel or distributed computation linear algebra Linear algebra labels Aug 3, 2018
@ViralBShah
Copy link
Member

Is there a decision to do something? Or close?

@vchuravy
Copy link
Member

So far we have assumed that people wanted to disable threading when using multiple processes but we could stop assuming that.

I for one are in favour of making things simpler and stop having Distributed change the number of threads in the BLAS library. It made sense when we didn't have robust multi-threading support and the expectation was that users would use Distributed to do single machine work. Machines since then have also gained cores even on laptops.

@vchuravy
Copy link
Member

Got done in #30004

@vchuravy vchuravy closed this Oct 31, 2020
@vchuravy vchuravy deleted the kc/break_linalg branch October 31, 2020 19:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
linear algebra Linear algebra parallelism Parallel or distributed computation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants