Preimage cache can get inadvertently messed up

[slwu89]

Apologies for the strange title, not sure what to call this. I am not sure if this behavior is a bug or not, if not, it's definitely worth documenting somewhere (and figuring out the general pattern which causes this behavior, to warn users). Funnily enough, this is something where when we have #17, it would naturally be avoided.

Consider the following code, it's natural for a user to grab all edges incident to some vertex along some hom, and then want to further filter that set by an attribute of the edge. Calling filter! with the bang somehow messes up the cached preimage such that when the user subsequently calls incident, the preimage has also been filtered. In the example, the second call to incident returns only [1,2], even though the mapping is still [1,1,1]. I pasted the invalidated preimage cache below this block.

using Catlab

wg = @acset WeightedGraph{Symbol} begin
    V=2
    E=3
    src=[1,1,1]
    tgt=[2,2,2]
    weight=[:a,:a,:b]
end

out = incident(wg, 1, :src)
filter!(e -> wg[e, :weight] == :a, out)

incident(wg, 1, :src)

where the issue is:

julia> wg.subparts[:src].m
ACSets.Mappings.VecMap{Int64, Vector{Int64}} with 3 entries:
  1 => 1
  2 => 1
  3 => 1

julia> wg.subparts[:src].pc
ACSets.PreimageCaches.StoredPreimageCache{Int64, Int64, ACSets.IndexUtils.SortedSet{Int64}, ACSets.Mappings.VecMap{ACSets.IndexUtils.SortedSet{Int64}, Vector{ACSets.IndexUtils.SortedSet{Int64}}}}(ACSets.Mappings.VecMap{ACSets.IndexUtils.SortedSet{Int64}, Vector{ACSets.IndexUtils.SortedSet{Int64}}}(1 => ACSets.IndexUtils.SortedSet{Int64}([1, 2])))

[epatters]

I think that, for performance reasons, incident returns a view onto the preimage cache, rather than a copy. This is a sensible default because if you're, say, traversing a graph you don't want to be making copies at every move. But it should certainly be documented somewhere if it's not.

We could also consider wrapping the vector in a struct sub-typing AbstractVector but enforcing the immutability. Then it would be impossible to get burned by this.

[slwu89]

Actually @epatters I just checked; it is documented. So it's user error on my part, happy to close this being the case.

[epatters]

No worries. If you want to open an issue about the last thing I mentioned, I am happy to consider it.