collect_or_id has problems with types from other packages

[slwu89]

Consider the following example, which is a decorated graph, where an attribute on vertices will store objects of type DenseAxisArray (from JuMP https://jump.dev/JuMP.jl/stable/manual/containers/#DenseAxisArray). When grabbing the attribute using chained indexing versus directly indexing, the type of the return object is different. Strangely, the chained access returns some internal data of the DenseAxisArray. See minimal ex below, on version Catlab v0.16.8

using Catlab, JuMP

@present SchGraphJuMP <: SchGraph begin
    Var::AttrType
    var::Attr(V,Var)
end

@abstract_acset_type AbstractGraphJuMP <: AbstractGraph

@acset_type GraphJuMP(SchGraphJuMP, index=[:src,:tgt]) <: AbstractGraphJuMP

graphopt = @acset GraphJuMP{JuMP.Containers.DenseAxisArray} begin
    V = 3
    E = 2
    src = [1,2]
    tgt = [2,3]
end

jumpmod = JuMP.Model()

for v in vertices(graphopt)
    var = @variable(
        jumpmod,
        [t ∈ 5:10],
    )
    graphopt[v,:var] = var
end

graphopt[2, (:tgt,:var)]
graphopt[2, [:tgt,:var]]
graphopt[3, :var]

Chained access returns:

6-element Vector{VariableRef}:
 _[19]
 _[20]
 _[21]
 _[22]
 _[23]
 _[24]

Directly accessing returns (as expected):

1-dimensional DenseAxisArray{VariableRef,1,...} with index sets:
    Dimension 1, 5:10
And data, a 6-element Vector{VariableRef}:
 _[19]
 _[20]
 _[21]
 _[22]
 _[23]
 _[24]

[slwu89]

Ok the issue has to do with collect_or_id, I changed the issue title to reflect that. See below, top works, bottom does not.

I'm not sure what to do here, it's slightly odd that the JuMP type is <: AbstractVector, but AFAIK there's also no real requirement that stuff which subtypes AbstractVector contains all its data in the iterable. This also means that other packages like https://github.com/JuliaArrays/AxisArrays.jl which store other stuff like offset indexing but still subtype AbstractVector might not behave well with this interface.

subpart(graphopt, 3, :var)
ACSets.ACSetInterface.collect_or_id(subpart(graphopt, 3, :var))

[slwu89]

The really heavy-handed approach would be to have a JuMP extension and then overrides for the various container types so they interact with collect_or_id properly. If nothing else better exists, I suggest that is a viable route, I am rather strongly convinced that a killer app for acsets is structuring optimization models, and operations research type models, generally.

[slwu89]

Thanks again @epatters and @kris-brown for the quick fix! Can confirm that the problem where I discovered this behavior now works flawlessly: https://github.com/slwu89/Orcas.jl/blob/main/src/BasicSchedule.jl#L278-L324