-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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 BLAS.get_num_threads #36360
Merged
Merged
add BLAS.get_num_threads #36360
Changes from 10 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
bea54cc
add BLAS.get_num_threads
jw3126 2de23cd
fix
jw3126 0e6f8fb
fix
jw3126 9f396c2
fix
jw3126 e84c258
fix
jw3126 9a0edcb
warn if get/set of num_bals_threads fails
jw3126 f6daa79
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 8b2c8c4
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 a15f851
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 826d8ff
fix
jw3126 ce61636
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 b38afaa
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 0ee0efa
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 8e4fedd
fix
jw3126 33a95d5
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 35cf5a6
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 bc370b4
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 dd455b5
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 abd2084
fix
jw3126 b8e9055
fix
jw3126 9eabcb6
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 920b90b
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 b011dc6
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 95ccdf9
Update stdlib/LinearAlgebra/test/blas.jl
jw3126 72ef30e
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 4cf2628
Update stdlib/LinearAlgebra/test/blas.jl
jw3126 bb69931
Update stdlib/LinearAlgebra/test/blas.jl
jw3126 06550d6
Update stdlib/LinearAlgebra/src/blas.jl
jw3126 b6aa076
improve docstrings
jw3126 c524c53
add to NEWS.md
jw3126 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems like an unusual API, I would prefer
set_num_threads()
instead ofset_num_threads(nothing)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to allow the pattern
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That will still work if you make the signature
set_num_threads(n=nothing)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To me allowing
nothing
is a hack to allow the above pattern on strange platforms. It is not a thing I would encourage or that I think needs more convenient syntax.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I imagine if you have code like
set_num_threads()
it is more likely you forgot to pass the number of threads, than that you really want to invoke thenothing
method.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we should define
set_num_threads() = set_num_threads(nothing)
. It would send a wrong message thatset_num_threads(nothing)
is somehow a reasonable default. But it's not. It is the last resort that exists only for supporting the rollback use case.But this is not clear from the current docstring. I think it's better to clarify this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps clearest to show the pattern which motivates this:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fredrikekre Does the argument here makes sense? It'd be nice if you can have a look at the docstring.