issue with _rem_part! and injective index

[slwu89]

I suspect this is a bug, although I think it is not directly related to cascading_rem_parts!, I discovered the problem by using that function. Basically, when there are uniquely indexed homs, cascading_rem_parts! leads to an "injectivity violated" error, but using the same schema with just standard indexing does not lead to this issue. I checked out the branch associated with #59 and can confirm that the behavior is still observed there.

Also, while I'm asking, in the docstring "Remove part and all parts incident to it, recursively.", does this mean that it only walks "backwards" along arrows in the presentation? (i.e. thinking of the word "incident" like how it is used in the incident function).

MWE here:

using ACSets
MySch = BasicSchema([:Thing,:Node,:Edge], [(:src,:Edge,:Node),(:tgt,:Edge,:Node),(:thing,:Thing,:Node)],[],[])

@acset_type MyData(MySch, index=[:src,:tgt], unique_index=[:thing])

mydata = @acset MyData begin
    Thing=3
    Node=3
    Edge=3
    thing=[1,2,3]
    src=[1,1,2]
    tgt=[1,2,3]
end

# errors
cascading_rem_parts!(mydata, :Node, 1)

@acset_type MyDataNoInjective(MySch, index=[:src,:tgt,:thing])

mydatanoinj = @acset MyDataNoInjective begin
    Thing=3
    Node=3
    Edge=3
    thing=[1,2,3]
    src=[1,1,2]
    tgt=[1,2,3]
end

# no error
cascading_rem_parts!(mydatanoinj, :Node, 1)

[slwu89]

Doing some digging reveals, as I thought, the problem is not in cascading_rem_parts!, the problem is actually here in _rem_part! https://github.com/AlgebraicJulia/ACSets.jl/blob/main/src/DenseACSets.jl#L577:

if haskey(acs.subparts[@ct f], last_part)
  set_subpart!(acs, part, (@ct f), subpart(acs, last_part, @ct f))
end
clear_subpart!(acs, last_part, @ct f)

We go into the line in the if statement and then hit the injectivity violated error there. This line is trying to get last_part's value of the subpart f, and then set part's value of the subpart f to that value. The problem with injective indices is that we haven't cleared out last_part's subpart, so we can't update part's subpart, since then injectivity is violated.

This seems to be a tricky problem.

[slwu89]

One thing I thought of doing was changing those lines to look like:

if haskey(acs.subparts[@ct f], last_part)
  last_part_f = subpart(acs, last_part, @ct f)
  clear_subpart!(acs, last_part, @ct f)
  set_subpart!(acs, part, (@ct f), last_part_f)
else
  clear_subpart!(acs, last_part, @ct f)
end

Which actually does work and give the correct results for the MWE for both injective and noninjective index. However then it ends up failing in tests because there's some tests which check for add_parts! to fail, and failure calls rem_part! again here https://github.com/AlgebraicJulia/ACSets.jl/blob/main/src/ACSetInterface.jl#L209 which somehow messes up the indexing when things aren't fully cleared out in the column implementation. I'll think about this later.