reverse(A, dims=...) doesn't work for Diagonal, Tridiagonal, etcetera about julia HOT 8 CLOSED

Hello, Is anyone currently working on this, if not I'd love to help out. Thanks

from julia.

Just go ahead!

from julia.

Since reverse calls Base._reverse(A, dims), you can overload the latter to dispatch on dims being an integer, e.g.

Base._reverse(A::Diagonal, dims::Integer) = reverse!(Matrix(A); dims)

Should be an easy PR — just add a few of these methods and a test that reverse(A, dims=x) == reverse(Matrix(A), dims=x).

You could also add specialized reverse methods for dims=: (i.e., reversing both dimensions) while you are at it, e.g.

Base._reverse!(A::Diagonal, dims::Colon) = (reverse!(A.diag); A)

(Note that this is the lower-level in-place variant, called by reverse.)

from julia.

hello, I was struggling a bit with solving this issue. If someone could tell me exactly where(ie in which files) I need to overload the method, that would help a lot, thanks!

from julia.

Please submit the fixes to the main Julia repo, where you would need to add the methods to "stdlib/LinearAlgebra/src/diagonal.jl" etc. I would recommend extending reverse directly instead of the internal _reverse, as the latter isn't public.

from julia.

Hi, I've been trying to solve this issue, Can you please clarify some doubts:
1)We do not want the error to be thrown anytime when we try to set off diagonal indices? or only for reverse?

In diagonal.jl:

function setindex!(D::Diagonal, v, i::Int, j::Int)
@BoundsCheck checkbounds(D, i, j)
if i == j
@inbounds D.diag[i] = v
elseif !iszero(v)
throw(ArgumentError(lazy"cannot set off-diagonal entry ($i, $j) to a nonzero value ($v)"))
end
return v
end

Should we convert everything from diagonal to matrix here and then set index?

2. Should we make changes in the file "base/arraymath.jl", if yes, should we import diagonal, Identity matrix etc to give in place of AbstractArray or AbstractMatrix?

reverse(A::AbstractArray; dims=:) = _reverse(A, dims)
reverse(A::Diagonal, dims=:) =_reverse(Matrix(A), dims)

Thank you

from julia.

You would need to make the changes in the LinearAlgebra standard library, which is where the types are defined, e.g. in diagonal.jl for handling the Diagonal type. For example, in diagonal.jl, you should only need to add three lines:

Base._reverse(A::Diagonal, dims) = reverse!(Matrix(A); dims)
Base._reverse(A::Diagonal, dims::Colon) = Diagonal(reverse(A.diag))
Base._reverse!(A::Diagonal, dims::Colon) = (reverse!(A.diag); A)

which gives

julia> reverse(Diagonal([1,2,3]))
3×3 Diagonal{Int64, Vector{Int64}}:
 3  ⋅  ⋅
 ⋅  2  ⋅
 ⋅  ⋅  1

julia> reverse(Diagonal([1,2,3]), dims=1)
3×3 Matrix{Int64}:
 0  0  3
 0  2  0
 1  0  0

The code for Tridiagonal etcetera should be very similar.

(Similarly, one could add optimized methods for SparseArrays.jl, but that's probably a separate issue. Optimized methods already exist for sparse arrays: JuliaSparse/SparseArrays.jl#450)

We do not want the error to be thrown anytime when we try to set off diagonal indices?

You're not trying to set any offdiagonal indices in the above methods.

reverse!(Diagonal([1,2,3]), dims=1) already correctly throws an error, and will still do so once the above methods are added.

from julia.

Got it, Thank you, I created a pull request, please review it

from julia.