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

UndefVarError: FinchProtocolError not defined #319

Closed
ericphanson opened this issue Dec 3, 2023 · 5 comments · Fixed by #348
Closed

UndefVarError: FinchProtocolError not defined #319

ericphanson opened this issue Dec 3, 2023 · 5 comments · Fixed by #348
Assignees
Milestone

Comments

@ericphanson
Copy link
Contributor

I've been trying to figure out how to transfer the result of a finch computation into a "usual" SparseMatrixCSC, which seemed to uncover a small bug:

julia> using Finch

julia> using Finch: AbstractFiber

julia> function transfer!(x, y::AbstractFiber)
           @finch begin
               x .= y
           end
       end
transfer! (generic function with 1 method)

julia> x = spzeros(2,2)
2×2 SparseMatrixCSC{Float64, Int64} with 0 stored entries:
       
       

julia> transfer!(x, f)
ERROR: UndefVarError: `FinchProtocolError` not defined
Stacktrace:
  [1] declare!(arr::SparseArraysExt.VirtualSparseMatrixCSC, ctx::Finch.LowerJulia, init::Finch.FinchNotation.FinchNode)
    @ SparseArraysExt ~/.julia/packages/Finch/R62UH/ext/SparseArraysExt.jl:60
  [2] declare!(tns::Finch.FinchNotation.FinchNode, ctx::Finch.LowerJulia, init::Finch.FinchNotation.FinchNode)
    @ Finch ~/.julia/packages/Finch/R62UH/src/tensors/combinators/roots.jl:20
  [3] lower(root::Finch.FinchNotation.FinchNode, ctx::Finch.LowerJulia, #unused#::Finch.DefaultStyle)
    @ Finch ~/.julia/packages/Finch/R62UH/src/lower.jl:112
  [4] (::Finch.LowerJulia)(root::Finch.FinchNotation.FinchNode, style::Finch.DefaultStyle)
    @ Finch ~/.julia/packages/Finch/R62UH/src/lower.jl:34
  [5] (::Finch.LowerJulia)(root::Finch.FinchNotation.FinchNode)
    @ Finch ~/.julia/packages/Finch/R62UH/src/lower.jl:32
  [6] (::Finch.var"#324#328")(ctx_3::Finch.LowerJulia)
    @ Finch ~/.julia/packages/Finch/R62UH/src/execute.jl:105
  [7] #130
    @ ~/.julia/packages/Finch/R62UH/src/lower.jl:15 [inlined]
  [8] contain(f::Finch.var"#130#131"{Dict{Any, Any}, Finch.var"#324#328", Finch.LowerJulia}, ctx::Finch.JuliaContext, task::Nothing)
    @ Finch ~/.julia/packages/Finch/R62UH/src/environment.jl:69
  [9] contain(f::Function, ctx::Finch.JuliaContext)
    @ Finch ~/.julia/packages/Finch/R62UH/src/environment.jl:63
 [10] contain(f::Finch.var"#324#328", ctx::Finch.LowerJulia; bindings::Dict{Any, Any}, kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ Finch ~/.julia/packages/Finch/R62UH/src/lower.jl:14
 [11] contain(f::Function, ctx::Finch.LowerJulia)
    @ Finch ~/.julia/packages/Finch/R62UH/src/lower.jl:13
 [12] (::Finch.var"#323#327")(ctx_2::Finch.LowerJulia)
    @ Finch ~/.julia/packages/Finch/R62UH/src/execute.jl:104
 [13] #130
    @ ~/.julia/packages/Finch/R62UH/src/lower.jl:15 [inlined]
 [14] contain(f::Finch.var"#130#131"{Dict{Any, Any}, Finch.var"#323#327", Finch.LowerJulia}, ctx::Finch.JuliaContext, task::Nothing)
    @ Finch ~/.julia/packages/Finch/R62UH/src/environment.jl:69
 [15] contain(f::Function, ctx::Finch.JuliaContext)
    @ Finch ~/.julia/packages/Finch/R62UH/src/environment.jl:63
 [16] contain(f::Finch.var"#323#327", ctx::Finch.LowerJulia; bindings::Dict{Any, Any}, kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ Finch ~/.julia/packages/Finch/R62UH/src/lower.jl:14
 [17] contain(f::Function, ctx::Finch.LowerJulia)
    @ Finch ~/.julia/packages/Finch/R62UH/src/lower.jl:13
 [18] lower_global(prgm::Finch.FinchNotation.FinchNode, ctx::Finch.LowerJulia)
    @ Finch ~/.julia/packages/Finch/R62UH/src/execute.jl:94
 [19] (::Finch.var"#321#322"{Symbol, DataType})(ctx_2::Finch.LowerJulia)
    @ Finch ~/.julia/packages/Finch/R62UH/src/execute.jl:82
 [20] #130
    @ ~/.julia/packages/Finch/R62UH/src/lower.jl:15 [inlined]
 [21] contain(f::Finch.var"#130#131"{Dict{Any, Any}, Finch.var"#321#322"{Symbol, DataType}, Finch.LowerJulia}, ctx::Finch.JuliaContext, task::Nothing)
    @ Finch ~/.julia/packages/Finch/R62UH/src/environment.jl:69
 [22] contain(f::Function, ctx::Finch.JuliaContext)
    @ Finch ~/.julia/packages/Finch/R62UH/src/environment.jl:63
 [23] contain(f::Finch.var"#321#322"{Symbol, DataType}, ctx::Finch.LowerJulia; bindings::Dict{Any, Any}, kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
    @ Finch ~/.julia/packages/Finch/R62UH/src/lower.jl:14
 [24] contain
    @ ~/.julia/packages/Finch/R62UH/src/lower.jl:13 [inlined]
 [25] #execute_code#320
    @ ~/.julia/packages/Finch/R62UH/src/execute.jl:79 [inlined]
 [26] execute_code
    @ ~/.julia/packages/Finch/R62UH/src/execute.jl:78 [inlined]
 [27] (::Finch.var"#316#319"{DataType, DataType})(ctx::Finch.JuliaContext)
    @ Finch ~/.julia/packages/Finch/R62UH/src/execute.jl:63
 [28] contain(f::Finch.var"#316#319"{DataType, DataType}, ctx::Finch.JuliaContext, task::Nothing)
    @ Finch ~/.julia/packages/Finch/R62UH/src/environment.jl:69
 [29] contain
    @ ~/.julia/packages/Finch/R62UH/src/environment.jl:63 [inlined]
 [30] macro expansion
    @ ~/.julia/packages/Finch/R62UH/src/execute.jl:62 [inlined]
 [31] (::Finch.var"#315#318"{DataType, DataType})()
    @ Finch ~/.julia/packages/Finch/R62UH/src/util.jl:71
 [32] #s355#314
    @ ~/.julia/packages/Finch/R62UH/src/util.jl:80 [inlined]
 [33] var"#s355#314"(::Any, ex::Any, opts::Any)
    @ Finch ./none:0
 [34] (::Core.GeneratedFunctionStub)(::Any, ::Vararg{Any})
    @ Core ./boot.jl:602
 [35] macro expansion
    @ ~/.julia/packages/Finch/R62UH/src/execute.jl:173 [inlined]
 [36] transfer!(x::SparseMatrixCSC{Float64, Int64}, y::Fiber{DenseLevel{Int64, SparseListLevel{Int64, Vector{Int64}, Vector{Int64}, ElementLevel{0, Int64, Int64, Vector{Int64}}}}})
    @ Main ./REPL[27]:2
 [37] top-level scope
    @ REPL[29]:1

I see the code in question is

function Finch.declare!(arr::VirtualSparseMatrixCSC, ctx::AbstractCompiler, init)
    throw(FinchProtocolError("Finch does not support writes to SparseMatrixCSC"))
end

so presumably I'm not allowed to do that, but the error object isn't defined in that scope.

@willow-ahrens
Copy link
Collaborator

willow-ahrens commented Dec 4, 2023

Thanks for filing the issue!
I see I have a few TODOs:

  • The protocol error needs to be imported from Finch in the sparsearrays extension package
  • We should create a function to convert to SparseMatrixCSC (Do we just pirate SparseMatrixCSC(::Fiber) and convert(:Type{SparseMatrixCSC}, ::Fiber)? Should we also do Array(::Fiber) or sparse(::Fiber))
  • We need docs for this too

@ericphanson
Copy link
Contributor Author

You probably know all this already, but FWIW I don't think it's actually piracy to add a method to the constructor or sparse since you own the type in question (Fiber). I would also probably skip convert to start since that will happen silently (e.g. push! into a Vector{SparseMatrixCSC{Int,Int}}() would convert) and require explicit conversion instead (xref: https://docs.julialang.org/en/v1/manual/conversion-and-promotion/#Conversion-vs.-Construction).

But adding methods to SparseMatrixCSC and Array and sparse sounds very helpful to me! I had tried all three of those when playing in the REPL 😅. Also it would be helpful to document if there is any aliasing in the docstrings (e.g. if SparseMatrixCSC always allocates fresh arrays or if for objects like Dense(SparseList(Element(0.0))) it could possibly re-use data).

@willow-ahrens
Copy link
Collaborator

Related: #288 #321 #320

@willow-ahrens
Copy link
Collaborator

@ericphanson I think the pr above fixes this issue, let me know if there's more to be done on this!

@willow-ahrens
Copy link
Collaborator

side note:
Finch currently uses the broadcast syntax as an initializer syntax for array variables, so the Finch code to copy into a sparse array would be

@finch begin
    x .= 0
    for j=_,i=_
        x[i, j] = y[i, j]
    end
end

For simple pointwise expressions, Finch also supports regular broadcast (outside of a @finch region)

#no @finch, just calling broadcast
x .= y

You could also call copyto!(x, y).

However, the above examples won't work when x is SparseMatrixCSC until we implement #320

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants