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.

Sign up for GitHub

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

Jump to bottom

remove unused private argument from BLAS.get_num_threads #38237

Merged
merged 1 commit into from
Nov 1, 2020
Merged

remove unused private argument from BLAS.get_num_threads #38237

merged 1 commit into from
Nov 1, 2020

Conversation

jw3126
Copy link
Contributor

@jw3126 jw3126 commented Oct 30, 2020

raminammour
raminammour reviewed Oct 30, 2020
View reviewed changes
Copy link
Contributor

@raminammour raminammour left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be:
get_num_threads(;_blas=guess_vendor())::Union{Int, Nothing} = _get_num_threads(_blas=_blas)

the keyword is used in the function just below if the user passes it.

@dkarrasch dkarrasch added the linear algebra Linear algebra label Oct 30, 2020
@jw3126
Copy link
Contributor Author

jw3126 commented Oct 30, 2020

Shouldn't this be:
get_num_threads(;_blas=guess_vendor())::Union{Int, Nothing} = _get_num_threads(_blas=_blas)

the keyword is used in the function just below if the user passes it.

No this keyword is private and should not be used. It exists only to allow a certain unit test.

@jw3126
Copy link
Contributor Author

jw3126 commented Oct 30, 2020

CI fail seems unrelated.

@dkarrasch dkarrasch merged commit 94e908f into JuliaLang:master Nov 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raminammour raminammour raminammour left review comments

@dkarrasch dkarrasch dkarrasch approved these changes

Assignees
No one assigned
Labels
linear algebra Linear algebra
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jw3126 @raminammour @dkarrasch