AlgebraicRewriting.jl tests are failing

[lukem12345]

An test using AlgebraicRewriting is failing:

Average Rewriting: Error During Test at D:\a\DiagrammaticEquations.jl\DiagrammaticEquations.jl\test\runtests.jl:23
  Got exception outside of a @test
  LoadError: MethodError: no method matching unions!(::DataStructures.IntDisjointSets{Int64}, ::Vector{Union{}})

  Closest candidates are:
    unions!(::DataStructures.IntDisjointSets, ::Vector{Int64})
     @ AlgebraicRewriting C:\Users\runneradmin\.julia\packages\AlgebraicRewriting\FCB57\src\categorical_algebra\CSets.jl:122

It is likely that the AlgebraicRewriting interface has updated recently and we need to update.

DecaTest2 = quote
  C₁::Form0{X}
  C₂::Form0{X}
  D₁::Form1{X}
  D₂::Form1{X}
  F::Form1{X}

  D₁ == d₀(C₁)
  D₁ == d₀(C₂)
  F == c₁(D₁)
  F == c₂(D₂)

end

Test2 = SummationDecapode(parse_decapode(DecaTest2))
Test2Res = average_rewrite(Test2)

The above call to average_rewrite fails.

[GeorgeR227]

Do we still want to support this functionality? This code hasn't been touched in well over a year.

[lukem12345]

The functionality yes. If it is easier to factor out AlgebraicRewriting with a more stable base Julia version then that's fine

On Thu, May 30, 2024, 2:29 PM GeorgeR227 @.***> wrote:

Do we still want to support this functionality? This code hasn't been touched in well over a year.

— Reply to this email directly, view it on GitHub https://github.com/AlgebraicJulia/DiagrammaticEquations.jl/issues/30#issuecomment-2140588045, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQYHBYMJQICAM3A7QC7NCLTZE5VYZAVCNFSM6AAAAABIRNMJBWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCNBQGU4DQMBUGU . You are receiving this because you were assigned.Message ID: @.***>

[GeorgeR227]

I set Rewriting version back down to 0.3.2 and the tests seems to pass. I'll make a PR to fix this.

[jpfairbanks]

I'm not attached to using the AlgebraicRewriting library to do these rewrites. Eventually pinning the version of AlgRewriting will hold back our usage of latest Catlab. At that point, we will need to rip out the AlgRewriting dependency.

Was the averaging rewrite worth using for accuracy or performance?

[GeorgeR227]

The average rewriting was never part of the compiling pipeline, it was always experimental. I'd be fine tearing it out.

[lukem12345]

Yeah it is not a necessary part of the compilation pipeline, but it is currently the only way we have of dealing with over-constrained physics besides picking a path through the computation graph at random

[quffaro]

For my clarification, is the objective of this ticket to replace the functionality defined in rewrite.jl to use vanilla Julia (or another package?)

[jpfairbanks]

I think we are still trying to decide whether we want to replace it with raw Julia that does the same thing by just editing the ACSets by adding and removing entries, or just kill the features entirely because they have never been adopted.