Skip to content

fix matrix multiplication interaction with HermOrSym and Diagonal #22474

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
Merged
merged 1 commit into from
Jun 24, 2017
Merged

fix matrix multiplication interaction with HermOrSym and Diagonal #22474

merged 1 commit into from
Jun 24, 2017

Conversation

fredrikekre
Copy link
Member

fredrikekre
fredrikekre commented Jun 22, 2017
View reviewed changes
base/linalg/diagonal.jl
@@ -150,9 +150,9 @@ end
(*)(D::Diagonal, B::AbstractTriangular) = A_mul_B!(D, copy(B))

(*)(A::AbstractMatrix, D::Diagonal) =
scale!(similar(A, promote_op(*, eltype(A), eltype(D.diag))), A, D.diag)
scale!(similar(A, promote_op(*, eltype(A), eltype(D.diag)), size(A)), A, D.diag)
Copy link
Member Author

@fredrikekre fredrikekre Jun 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This fix is a bit dirty; currently similar(A::Symmetric) return a Matrix iff you supply both a new element type and size, can we rely on this behavior?
Ref: #19016 #15198 and #13731

Copy link
Member Author

Choose a reason for hiding this comment

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

Seem like this is used elsewhere also, so I think this is OK.

Copy link
Contributor

Choose a reason for hiding this comment

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

There are some ongoing open API issues between similar vs "copystructure" but for now I think it's okay

@fredrikekre fredrikekre mentioned this pull request Jun 22, 2017
Merged
@kshyatt kshyatt added the linear algebra Linear algebra label Jun 22, 2017
Sacha0
Sacha0 approved these changes Jun 23, 2017
View reviewed changes
Copy link
Member

@Sacha0 Sacha0 left a comment

Choose a reason for hiding this comment

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

lgtm! :)

@andreasnoack andreasnoack merged commit 0990d9b into JuliaLang:master Jun 24, 2017
@fredrikekre
Copy link
Member Author

Thanks for reviewing/merging.

@fredrikekre fredrikekre deleted the fe/sym-herm-diag branch June 25, 2017 02:16
DrTodd13 pushed a commit to IntelLabs/julia that referenced this pull request Jun 26, 2017
@fredrikekre @DrTodd13
fix matrix multiplication interaction with HermOrSym and Diagonal (Ju…
ee6f749 
…liaLang#22474)
ararslan pushed a commit that referenced this pull request Sep 11, 2017
@fredrikekre @ararslan
fix matrix multiplication interaction with HermOrSym and Diagonal (#2…
12b4d4c 
…2474)

Ref #22474
(cherry picked from commit 0990d9b)
ararslan pushed a commit that referenced this pull request Sep 13, 2017
@fredrikekre @ararslan
fix matrix multiplication interaction with HermOrSym and Diagonal (#2…
d84cbd3 
…2474)

(cherry picked from commit 0990d9b)
vtjnash pushed a commit that referenced this pull request Sep 14, 2017
@fredrikekre @vtjnash
fix matrix multiplication interaction with HermOrSym and Diagonal (#2…
94a8bd8 
…2474)

(cherry picked from commit 0990d9b)
ararslan pushed a commit that referenced this pull request Sep 15, 2017
@fredrikekre @ararslan
fix matrix multiplication interaction with HermOrSym and Diagonal (#2…
21dcfec 
…2474)

(cherry picked from commit 0990d9b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tkelman tkelman tkelman left review comments

@Sacha0 Sacha0 Sacha0 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.
7 participants
@fredrikekre @Sacha0 @tkelman @andreasnoack @kshyatt @KristofferC @ararslan