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

rdiv!(randn(4,4), I(4)) fails #40887

Closed
olof3 opened this issue May 20, 2021 · 6 comments · Fixed by #40942
Closed

rdiv!(randn(4,4), I(4)) fails #40887

olof3 opened this issue May 20, 2021 · 6 comments · Fixed by #40942
Labels
good first issue Indicates a good issue for first-time contributors to Julia linear algebra Linear algebra

Comments

@olof3
Copy link
Contributor

olof3 commented May 20, 2021

julia> rdiv!(randn(4,4), I(4))
ERROR: MethodError: no method matching rdiv!(::Matrix{Float64}, ::Diagonal{Bool, Vector{Bool}})

There is an rdiv! method but it requires the arguments to have the same eltype, i.e.,
rdiv!(randn(4,4), 1.0I(4)) works

I also noticed that there are two ldiv! methods for Diagonal and Matrix. ldiv!(D::Diagonal, B::StridedVecOrMat) and
ldiv!(D::Diagonal{T}, V::AbstractMatrix{T}). This seems redundant?

@dkarrasch dkarrasch added linear algebra Linear algebra good first issue Indicates a good issue for first-time contributors to Julia labels May 20, 2021
@dkarrasch
Copy link
Member

I also noticed that there are two ldiv! methods for Diagonal and Matrix.

This might have been resolved meanwhile (#38484 ?).

@olof3
Copy link
Contributor Author

olof3 commented May 20, 2021

Great, yes the redundancy issue seems to have been resolved.

@Pramodh-G
Copy link
Contributor

Can this issue be closed then?

@olof3
Copy link
Contributor Author

olof3 commented May 20, 2021

Can this issue be closed then?

No, the main issue has not been resolved.

@ArunS-tack
Copy link
Contributor

Can this issue be closed then?

No, the main issue has not been resolved.

is this to be solved in diagonal.jl file?

@olof3
Copy link
Contributor Author

olof3 commented May 24, 2021

is this to be solved in diagonal.jl file?

I would say that's the place. I guess it should be made consistent with ldiv! (although not the AbstractVecOrMat specification) https://github.com/mcognetta/julia/blob/2dbedeb047ae8b322fd97d3725d5170d497b1ff3/stdlib/LinearAlgebra/src/diagonal.jl#L590

Would probably also be a good idea to move the ldiv! and rdiv! methods to the same place in that file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Indicates a good issue for first-time contributors to Julia linear algebra Linear algebra
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants