-
-
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
fix fallback for scaling blas number with array #17739
Conversation
@@ -18,6 +20,8 @@ function scale!{T<:BlasFloat}(X::Array{T}, s::T) | |||
X | |||
end | |||
|
|||
scale!{T<:BlasFloat}(s::T, X::Array{T}) = scale!(X, s) |
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.
why does this method exist? really strange to be sometimes mutating the second input, but usually the first
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 to be symmetric like multiplication?
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.
Because multiplication is not always commutative.
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 also thought it was weird. I guess it is there for non commutative numbers in the general case.
Edit: Beaten to the punch
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.
mutating methods need to be consistent about what they're mutating.
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.
No they don't. We don't agree here and for those of us who use this method, it doesn't cause trouble.
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.
It's mutating the array, whichever argument that is.
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.
Added to #16772. Being explicit with 3 arguments seems like a much more predictable API when you're writing generic code that has to deal with various combinations of input types.
Apple seemed to got stuck on something unrelated. Since this should be fairly uncontroversial I'm gonna click the scary button for the first time. 🎉 |
Also adds a fast path for special case of zero and one.
JuliaLang/LinearAlgebra.jl#351