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

Exponentiation of Diagonal{*Range} fails #40886

Closed
dkarrasch opened this issue May 20, 2021 · 8 comments · Fixed by #40991
Closed

Exponentiation of Diagonal{*Range} fails #40886

dkarrasch opened this issue May 20, 2021 · 8 comments · Fixed by #40991
Labels
domain:linear algebra Linear algebra good first issue Indicates a good issue for first-time contributors to Julia kind:bug Indicates an unexpected problem or unintended behavior

Comments

@dkarrasch
Copy link
Member

julia> using LinearAlgebra; D = Diagonal(1:3)
3×3 Diagonal{Int64, UnitRange{Int64}}:
 1    
   2  
     3

julia> D^2
ERROR: MethodError: no method matching Vector{Int64}(::Diagonal{Int64, UnitRange{Int64}})
Closest candidates are:
  Array{T, N}(::AbstractArray{S, N}) where {T, N, S} at array.jl:540
  Vector{T}() where T at boot.jl:467
  Vector{T}(::Core.Compiler.AbstractRange{T}) where T at range.jl:1042
  ...
Stacktrace:
 [1] convert(#unused#::Type{Vector{Int64}}, a::Diagonal{Int64, UnitRange{Int64}})
   @ Base ./array.jl:532
 [2] Diagonal{Int64, Vector{Int64}}(diag::Diagonal{Int64, UnitRange{Int64}})
   @ LinearAlgebra /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.6/LinearAlgebra/src/diagonal.jl:10
 [3] convert(T::Type{Diagonal{Int64, Vector{Int64}}}, m::Diagonal{Int64, UnitRange{Int64}})
   @ LinearAlgebra /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.6/LinearAlgebra/src/special.jl:53
 [4] to_power_type(x::Diagonal{Int64, UnitRange{Int64}})
   @ Base ./intfuncs.jl:240
 [5] power_by_squaring(x_::Diagonal{Int64, UnitRange{Int64}}, p::Int64)
   @ Base ./intfuncs.jl:255
 [6] ^(A::Diagonal{Int64, UnitRange{Int64}}, p::Int64)
   @ LinearAlgebra /Users/julia/buildbot/worker/package_macos64/build/usr/share/julia/stdlib/v1.6/LinearAlgebra/src/dense.jl:447
 [7] macro expansion
   @ ./none:0 [inlined]
 [8] literal_pow(f::typeof(^), x::Diagonal{Int64, UnitRange{Int64}}, #unused#::Val{2})
   @ Base ./none:0
 [9] top-level scope
   @ REPL[41]:1

It fails because the Diagonal{T,V} constructor is being called with a diagonal matrix as argument (item [2] in the stacktrace above), whereas the constructor here expects a vector.

Originally posted by @daanhb in #40831 (comment)

@dkarrasch
Copy link
Member Author

Should be easily fixable by

(^)(D::Diagonal, a::Number) = Diagonal(D.diag .^ a)

@dkarrasch dkarrasch added kind:bug Indicates an unexpected problem or unintended behavior good first issue Indicates a good issue for first-time contributors to Julia domain:linear algebra Linear algebra labels May 20, 2021
@ArunS-tack
Copy link
Contributor

Should be easily fixable by

(^)(D::Diagonal, a::Number) = Diagonal(D.diag .^ a)

In which file should this be added/edited?

@andreasnoack
Copy link
Member

andreasnoack commented May 23, 2021 via email

@mcabbott
Copy link
Contributor

This fails on 1.6, but seems to work on master:

julia> D = Diagonal(1:3);

julia> D ^ 2
3×3 Diagonal{Int64, Vector{Int64}}:
 1  ⋅  ⋅
 ⋅  4  ⋅
 ⋅  ⋅  9

julia> let p = 3; D ^ p; end
3×3 Diagonal{Int64, Vector{Int64}}:
 1  ⋅   ⋅
 ⋅  8   ⋅
 ⋅  ⋅  27

@ArunS-tack
Copy link
Contributor

This fails on 1.6, but seems to work on master:

julia> D = Diagonal(1:3);

julia> D ^ 2
3×3 Diagonal{Int64, Vector{Int64}}:
 1  ⋅  ⋅
 ⋅  4  ⋅
 ⋅  ⋅  9

julia> let p = 3; D ^ p; end
3×3 Diagonal{Int64, Vector{Int64}}:
 1  ⋅   ⋅
 ⋅  8   ⋅
 ⋅  ⋅  27

How do i test it? :/ I m new to all this. I am still trying to figure out how to Test the changes before making a PR.

@mcabbott
Copy link
Contributor

You can just paste exactly what you quoted into the REPL. You can download a the "nightly" version of Julia from the main download page, and keep this alongside 1.6 on your computer, to try things out.

Actual tests for a PR, you should find the right file in test/ & add some lines (or a new testset) there. But this too you can try out locally, by just using Test and pasting your new tests (and any nearby ones you are worried about).

@ArunS-tack
Copy link
Contributor

ArunS-tack commented May 23, 2021

You can just paste exactly what you quoted into the REPL. You can download a the "nightly" version of Julia from the main download page, and keep this alongside 1.6 on your computer, to try things out.

Actual tests for a PR, you should find the right file in test/ & add some lines (or a new testset) there. But this too you can try out locally, by just using Test and pasting your new tests (and any nearby ones you are worried about).

In this case, paste the whole diagonal.jl code into repl? And I did go to test/ but there was no file named diagonal.jl.

@mcabbott
Copy link
Contributor

No, you can literally paste my message in (the quoted part, for which github gives you a copy button). It is smart enough to remove the julia> prompts & ignore the answers.

This is the test file: https://github.com/JuliaLang/julia/blob/master/stdlib/LinearAlgebra/test/diagonal.jl#L777 . You should be able to copy & run bits of it, without running all of Julia's tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:linear algebra Linear algebra good first issue Indicates a good issue for first-time contributors to Julia kind:bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants