AlgebraicJulia / DiagrammaticEquations.jl

MIT License
12 stars 1 forks source link

Expression level rewriting #69

Closed GeorgeR227 closed 1 month ago

GeorgeR227 commented 2 months ago

This PR is meant to create a back-and-forth between ACSets, which excel at information propagation (typing, overloading, etc.) and Symbolics, which excel at expression rewriting (open_operators, optimizations, etc.).

This should remove the need for our graph rewriting functions and make the workflow to add new rewrites into the system much more faster/cleaner.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 90.00000% with 11 lines in your changes missing coverage. Please review.

Project coverage is 82.01%. Comparing base (0a314a8) to head (67079cb).

Files with missing lines Patch % Lines
src/acset2symbolic.jl 91.83% 4 Missing :warning:
src/deca/ThDEC.jl 55.55% 4 Missing :warning:
src/graph_traversal.jl 94.28% 2 Missing :warning:
src/sym_rewrite.jl 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## symbolicutilsinterop #69 +/- ## ======================================================== + Coverage 74.90% 82.01% +7.10% ======================================================== Files 16 19 +3 Lines 1068 1173 +105 ======================================================== + Hits 800 962 +162 + Misses 268 211 -57 ``` | [Flag](https://app.codecov.io/gh/AlgebraicJulia/DiagrammaticEquations.jl/pull/69/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AlgebraicJulia) | Coverage Δ | | |---|---|---| | [](https://app.codecov.io/gh/AlgebraicJulia/DiagrammaticEquations.jl/pull/69/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AlgebraicJulia) | `82.01% <90.00%> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AlgebraicJulia#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

lukem12345 commented 2 months ago

When roundtripping to the SummationDecapode is complete, it would be good to test that the results in test/openoperators.jl give the same results up to renaming etc

GeorgeR227 commented 2 months ago

I've rebased this branch to work off of PR #64 since the code here will be used for the rewriting using only Sort.

lukem12345 commented 1 month ago

@quffaro It looks like your commit 2b3198f removed the check for $$\partial_t$$ in extract_symexprs, but your merge 367414d seems to have possibly errantly put the check back. Which behavior do you want. Is there a test that passes or fails depending on whether this is checked?

GeorgeR227 commented 1 month ago

@lukem12345 We wanted to remove this check since it lead to a lot of special code to handle the final ACSet generation. I've removed it in commit https://github.com/AlgebraicJulia/DiagrammaticEquations.jl/pull/69/commits/5b84cc8a93097832e2a0fa492bba913f17904bba and added the appropriate tests to ensure it's proper functionality.