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

[ITensors] [BUG] nnz methods duplicated with SparseArrays #1581

Closed
dgleich opened this issue Nov 13, 2024 · 4 comments · Fixed by #1582
Closed

[ITensors] [BUG] nnz methods duplicated with SparseArrays #1581

dgleich opened this issue Nov 13, 2024 · 4 comments · Fixed by #1582
Labels
bug Something isn't working ITensors Issues or pull requests related to the `ITensors` package.

Comments

@dgleich
Copy link

dgleich commented Nov 13, 2024

Description of bug

I believe that the nnz function for ITensors.jl should extend the one from SparseArrays. But this doesn't happen.

Minimal code demonstrating the bug or unexpected behavior

using ITensors, SparseArrays
A = sprand(10, 10, 0.5)
nnz(A) 

This gives

julia> nnz(A)
ERROR: UndefVarError: `nnz` not defined in `Main`
Suggestion: check for spelling errors or missing imports.
Hint: a global variable of this name also exists in SparseArrays.
Hint: a global variable of this name also exists in NDTensors.
Hint: a global variable of this name also exists in ITensors.

Looking at the methods, it seems like this is probably an oversight:


julia> methods(ITensors.nnz)
# 5 methods for generic function "nnz" from NDTensors:
 [1] nnz(T::ITensor)
     @ ITensors ~/.julia/packages/ITensors/GkEpm/src/itensor.jl:855
 [2] nnz(::NDTensors.EmptyStorage)
     @ ~/.julia/packages/NDTensors/OehsJ/src/empty/empty.jl:69
 [3] nnz(bofs::Dictionaries.Dictionary{Block{N}, Int64} where N, inds)
     @ ~/.julia/packages/NDTensors/OehsJ/src/blocksparse/blockoffsets.jl:60
 [4] nnz(T::NDTensors.Tensor)
     @ ~/.julia/packages/NDTensors/OehsJ/src/tensor/tensor.jl:310
 [5] nnz(S::NDTensors.TensorStorage)
     @ ~/.julia/packages/NDTensors/OehsJ/src/tensorstorage/tensorstorage.jl:101

julia> methods(SparseArrays.nnz)
# 15 methods for generic function "nnz" from SparseArrays:
  [1] nnz(S::LinearAlgebra.LowerTriangular{<:Any, <:SparseArrays.AbstractSparseMatrixCSC})
     @ /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/sparsematrix.jl:222
  [2] nnz(x::SubArray{Tv, 1, <:AbstractSparseVector{Tv, Ti}, Tuple{Base.Slice{Base.OneTo{Int64}}}, false} where {Tv, Ti})
     @ /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/sparsevector.jl:119
  [3] nnz(x::SubArray{Tv, 1, <:AbstractSparseVector{Tv, Ti}, <:Tuple{AbstractUnitRange}, false} where {Tv, Ti})
     @ /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/sparsevector.jl:120
  [4] nnz(x::SubArray{Tv, 1, <:SparseArrays.AbstractSparseMatrixCSC{Tv, Ti}, Tuple{Base.Slice{Base.OneTo{Int64}}, Int64}, false} where {Tv, Ti})
     @ /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/sparsevector.jl:115
  [5] nnz(S::SubArray{Tv, 2, <:SparseArrays.AbstractSparseMatrixCSC{Tv, Ti}, Tuple{Base.Slice{Base.OneTo{Int64}}, I}} where {Tv, Ti, I<:(AbstractVector{<:Integer})})
     @ /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/sparsematrix.jl:223
  [6] nnz(lu::SparseArrays.UMFPACK.UmfpackLU)
     @ SparseArrays.UMFPACK /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/solvers/umfpack.jl:938
  [7] nnz(A::SparseArrays.CHOLMOD.Sparse{<:Union{Float32, Float64, ComplexF64, ComplexF32}, Int64})
     @ SparseArrays.CHOLMOD /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/solvers/cholmod.jl:601
  [8] nnz(A::SparseArrays.CHOLMOD.Sparse{<:Union{Float32, Float64, ComplexF64, ComplexF32}, Int32})
     @ SparseArrays.CHOLMOD /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/solvers/cholmod.jl:601
  [9] nnz(x::SparseArrays.AbstractCompressedVector)
     @ /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/sparsevector.jl:114
 [10] nnz(S::SparseArrays.AbstractSparseMatrixCSC)
     @ /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/sparsematrix.jl:218
 [11] nnz(S::Base.ReshapedArray{<:Any, 1, <:SparseArrays.AbstractSparseMatrixCSC})
     @ /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/sparsematrix.jl:219
 [12] nnz(S::LinearAlgebra.UpperTriangular{<:Any, <:SparseArrays.AbstractSparseMatrixCSC})
     @ /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/sparsematrix.jl:221
 [13] nnz(F::SparseArrays.CHOLMOD.Factor)
     @ SparseArrays.CHOLMOD /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/solvers/cholmod.jl:1268
 [14] nnz(a::NDTensors.SparseArrayInterface.AbstractSparseArray)
     @ NDTensors.SparseArrayInterface ~/.julia/packages/NDTensors/OehsJ/src/lib/SparseArrayInterface/src/abstractsparsearray/SparseArrayInterfaceSparseArraysExt.jl:6
 [15] nnz(S::Union{LinearAlgebra.Adjoint{var"#s15", var"#s14"}, LinearAlgebra.Transpose{var"#s15", var"#s14"}} where {var"#s15", var"#s14"<:SparseArrays.AbstractSparseMatrixCSC})
     @ /Applications/Julia-1.11.app/Contents/Resources/julia/share/julia/stdlib/v1.11/SparseArrays/src/sparsematrix.jl:220

Version information

  • Output from versioninfo():
julia> versioninfo()
Julia Version 1.11.0
Commit 501a4f25c2b (2024-10-07 11:40 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: macOS (x86_64-apple-darwin22.4.0)
  CPU: 16 × Intel(R) Xeon(R) W-2140B CPU @ 3.20GHz
  WORD_SIZE: 64
  LLVM: libLLVM-16.0.6 (ORCJIT, skylake-avx512)
Threads: 1 default, 0 interactive, 1 GC (on 16 virtual cores)
  • Output from using Pkg; Pkg.status("ITensors"):
julia> using Pkg; Pkg.status("ITensors")
⌅ [9136182c] ITensors v0.7.1
@dgleich dgleich added bug Something isn't working ITensors Issues or pull requests related to the `ITensors` package. labels Nov 13, 2024
@mtfishman
Copy link
Member

Thanks for pointing that out, that sounds right. We were changing over the style of our code to use explicit imports, probably this got messed up during that process.

@dgleich
Copy link
Author

dgleich commented Nov 14, 2024

There's also permute which exists in SparseArrays too. But I'm not sure if you deliberately chose not extend that one as SparseArrays.permute does a row/column permutation whereas your permute reorders the indices.

@mtfishman
Copy link
Member

mtfishman commented Nov 14, 2024

There's also permute which exists in SparseArrays too. But I'm not sure if you deliberately chose not extend that one as SparseArrays.permute does a row/column permutation whereas your permute reorders the indices.

Thanks for the heads up about that. I feel less comfortable overloading SparseArrays.permute since as you identified ITensors.permute permutes/aligns the dimensions/indices, so they are pretty different concepts. Additionally, both are bad names for their respective operations, I don't understand why SparseArrays.permute(A, [4, 3, 2, 1], [1, 2, 3, 4]) even exists as an API since it should be the same as A[[4, 3, 2, 1], [1, 2, 3, 4]]. Additionally, ITensors.permute is a bad name since the input isn't a permutation, and we plan to rename it to something like align or aligndims, inspired by the existing/proposed APIs in NamedDims.jl, NamedPlus.jl, and PyTorch.

So in that case, I would recommend being explicit and qualifying the names with the modules, i.e. write ITensors.permute or SparseArrays.permute explicitly as needed (see https://docs.julialang.org/en/v1/manual/modules/#Handling-name-conflicts, https://github.com/ericphanson/ExplicitImports.jl).

@dgleich
Copy link
Author

dgleich commented Nov 14, 2024

Thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ITensors Issues or pull requests related to the `ITensors` package.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants