Roe cleanup by quffaro · Pull Request #161 · AlgebraicJulia/GATlab.jl

quffaro · 2024-07-23T20:12:25Z

No description provided.

olynch

General comments:

We should not be using the GATlab nomenclature of ThDEC/models, to avoid confusion
Let's try to make folder structure of test/ mimic the folder structure of src/.

olynch · 2024-07-23T20:13:24Z

@@ -0,0 +1,222 @@
+# """


Maybe this should just be called "roe.jl" and be included into module.jl with no surrounding module?

olynch · 2024-07-23T20:13:56Z

+
+Struct containing a Function and the vector of Sorts it requires.
+"""
+@struct_hash_equal struct TypedApplication


This, and other things that have AbstractSort parameters, should be typed based on a subtype of AbstractSort.

olynch · 2024-07-23T20:16:19Z

+
+"""   fix_functions(e)::Union{Symbol, Expr}
+
+Traverses the AST of an expression, replacing the head of :call expressions to its name, a Symbol.


Add to this comment that fix_functions is used for the show method of Vars

olynch · 2024-07-23T20:18:13Z

+
+"""    fresh!(roe::Roe, sort::AbstractSort, name::Symbol)::Var{sort}
+
+Creates a new ("fresh") variable in a Roe given a sort and a name.


Should be something like:

Creates a new variable in a Roe. Specifically, appends a new RootVar with given sort and name to the Roe, adds that RootVar to the e-graph, and returns a Var wrapper around the new e-graph id, with type parameter given by the sort.

olynch · 2024-07-23T20:20:21Z

@@ -0,0 +1,137 @@
+module SSAExtract


Maybe just call this SSAs. And then all the things prefixed with SSA (i.e. SSAVar, etc.) can be SSAs.Var, etc.

olynch · 2024-07-23T20:33:18Z

+    end
+end
+
+function ι(s1::Sort, s2::Sort)


Either complete these or add TODO comments

olynch · 2024-07-23T20:36:26Z

+import Base: +, -, *
+
+# These operations create calls on a common egraph. We validate the signature by dispatching the operation on the types using methods we defined in Signature.
+


for binop in [+, *, -, \wedge, ...] @eval begin function $binop(v1::Var{s1}, v2::Var{s2}) where {s1, s2} v1.roe === v2.roe || error("Cannot add variables from different graphs") s = $binop(s1, s2) Var{s}(v1.roe, addcall!(v1.roe.graph, $binop, (v1.id, v2.id))) end export $binop end end

This might be the blessed way to do it (also add the overloads with Number) -- we should doublecheck that this will run during precompilation though!

This looks fine to me. Julia's Base does this for op in [+,*...] a lot to define all the possible methods between different numeric types.

olynch · 2024-07-23T20:38:54Z

+Given a matrix and a hodge star (DiagonalHodge() or GeometricHodge()), this returns a lookup dictionary between operators (as TypedApplications) and their corresponding matrices.
+
+"""
+function precompute_matrices(sd, hodge)::Dict{TypedApplication, Any}


Maybe this should be called something more generic, like "create_dynamic_model" or something? Basically, what this is doing is creating a Dict that stores a model of the theory of DEC. We can workshop that name though.

olynch · 2024-07-23T20:42:53Z

+        ::RootVar => rootvar_lookup[expr.head]
+        ::Number => expr.head
+        _ => begin
+          op = operator_lookup[TA(expr.head, first.(expr.args))]


op = get(operator_lookup, TA(expr.head, first.(expr.args)), op)

olynch · 2024-07-23T20:46:34Z

+export derivative_cost
+
+
+""" vfield :: (Decaroe -> (StateVars, ParamVars)) -> VectorFieldFunction


Right now this is hacked to assume that there only one state variable and only one parameter variable, because we haven't integrated with ComponentArrays yet: we should integrate with ComponentArrays.

"integrating with ComponentArrays" should just mean that you assume the state variable and parameter variables can be accessed by symbolic keys, and that what you get back from indexing will be either a vector or a scalar value so you use broadcasted operations when necessary.

It is a pretty light lift from the users perspective.

KevinDCarlson · 2024-07-23T23:18:36Z

+end
+
+# we can reuse the mesh and operator lookup
+_vf = vfield(new_heat_equation, operator_lookup)


this one won't run for me

_vf = vfield(new_heat_equation, operator_lookup) # XXX: brooken ERROR: KeyError: key Scalar() * PrimalForm(0) not found Stacktrace: [1] getindex @ ./dict.jl:477 [inlined] [2] (::DEC.Models.ThDEC.var"#toexpr#15"{Dict{TypedApplication, Any}, Dict{RootVar, Union{Expr, Symbol}}})(expr::SSAExpr) @ DEC.Models.ThDEC ~/.julia/dev/GATlab/DEC/src/models/ThDEC/ThDEC.jl:97 [3] (::DEC.Models.ThDEC.var"#7#17")(::Any) @ DEC.Models.ThDEC ~/.julia/dev/GATlab/DEC/src/models/ThDEC/ThDEC.jl:126 [4] iterate @ ./generator.jl:48 [inlined] [5] collect_to! @ ./array.jl:838 [inlined] [6] collect_to_with_first! @ ./array.jl:816 [inlined] [7] collect(itr::Base.Generator{Base.Iterators.Enumerate{Vector{SSAExpr}}, DEC.Models.ThDEC.var"#7#17"}) @ Base ./array.jl:790 [8] map @ ./abstractarray.jl:3402 [inlined] [9] vfield(model::Any, operator_lookup::Dict{TypedApplication, Any}) @ DEC.Models.ThDEC ~/.julia/dev/GATlab/DEC/src/models/ThDEC/ThDEC.jl:125 [10] top-level scope @ REPL[26]:1

Right, @olynch and I reviewed this today and decided we need to just tweak the code to use Julia's * operation. This and the other comments should be resolved tomorrow

jpfairbanks · 2024-07-29T13:47:15Z

High level thought @olynch, we discussed on Thursday in lab meeting that the Roe stuff is just based on Metatheory.jl so it doesn't really need Gatlab integration yet. Should this PR end up in DiagrammaticEquations.jl?

KevinDCarlson · 2024-07-29T18:23:37Z

High level thought @olynch, we discussed on Thursday in lab meeting that the Roe stuff is just based on Metatheory.jl so it doesn't really need Gatlab integration yet. Should this PR end up in DiagrammaticEquations.jl?

That’d be sick if it’s reasonable

…rogress on docs

lukem12345 · 2024-08-02T21:25:20Z

+    # :Δᵈ₁ => Δᵈ(Val{1},sd)
+
+    # # Musical Isomorphisms
+    TA(♯, Sort[PrimalForm(1)]) => Decapodes.dec_♯_p(sd), # Primal(1) -> PVField


As discussed on Zulip and offline, this dict needs to be refactored to emit functions based on input and output types both. This is to accommodate e.g. the fact that there is a $$\flat_{pp}$$ and a $$\flat_{pd}$$. See Figure 5.2.

At this level, the output type is determined by the input types, so I'm not really sure what this means.

In the DEC, an e.g. sharp operator can be defined which takes a primal form and outputs a primal vector field, while another sharp operator takes a primal form and emits a dual vector field. Both of these can coexist.

The appearance of dual complexes leads to a proliferation of the operators in the discrete theory. For example there are
primal-primal, primal-dual etc. versions of many operators. This is of course unique to the discrete side.

From hirani

lukem12345 · 2024-08-02T21:53:45Z

+  function TypedApplication(head::Function, sorts::Vector{Sort}) where Sort
+    new{Sort}(head, sorts)
+  end
+end


@struct_hash_equal struct TypedApplication{Sort<:AbstractSort} head::Function insorts::Vector{Sort} outsorts::Vector{Sort} function TypedApplication(head::Function, sorts::Vector{Sort}) where Sort new{Sort}(head, sorts) end end

lukem12345 · 2024-08-02T21:55:40Z

+    TA(♯, Sort[PrimalForm(1)]) => Decapodes.dec_♯_p(sd), # Primal(1) -> PVField
+    TA(♯, Sort[DualForm(1)]) => Decapodes.dec_♯_d(sd), # Dual(1) -> DVField
+
+    TA(♭, Sort[DualVF()]) => Decapodes.dec_♭(sd), # DVField -> Primal(1)


TA(♭, Sort[DualVF()], Sort[Primal(1)]) => Decapodes.dec_♭(sd)

Und so weiter

lukem12345 · 2024-08-02T21:57:42Z

+
+"""
+function create_dynamic_model(sd, hodge)::Dict{TypedApplication, Any}
+  Dict{TypedApplication, Any}(


We need to special-case sums of more than 32 forms.

We need to not do sums two-at-a-time.

lukem12345 · 2024-08-02T21:58:29Z

+Given a matrix and a hodge star (DiagonalHodge() or GeometricHodge()), this returns a lookup dictionary between operators (as TypedApplications) and their corresponding matrices.
+
+"""
+function create_dynamic_model(sd, hodge)::Dict{TypedApplication, Any}


Purposes of using a Dict here vs. an @match block?

I think the blessed way to do this is going to actually be to create a memoized function/lazy dict. Best of both worlds: we don't have to compute operators that we don't need, and we don't have to compute operators multiple times.

codecov · 2026-03-31T08:28:54Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.37%. Comparing base (13a5cee) to head (175a9e6).

Additional details and impacted files

@@              Coverage Diff               @@
##           compute-graph     #161   +/-   ##
==============================================
  Coverage          84.37%   84.37%           
==============================================
  Files                 42       42           
  Lines               2873     2873           
==============================================
  Hits                2424     2424           
  Misses               449      449

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

first pass at organizing code

aa6b2f9

quffaro requested a review from olynch July 23, 2024 20:12

submitting for cleanup

f15f27b

olynch requested changes Jul 23, 2024

View reviewed changes

olynch changed the base branch from main to compute-graph July 23, 2024 20:50

KevinDCarlson reviewed Jul 23, 2024

View reviewed changes

quffaro added 3 commits August 1, 2024 14:26

completing code review

94ef1ce

begun rewriting work

8358b74

prototype of typed rewriting for theories and continued with little p…

175a9e6

…rogress on docs

lukem12345 reviewed Aug 2, 2024

View reviewed changes


		""" fix_functions(e)::Union{Symbol, Expr}

		Traverses the AST of an expression, replacing the head of :call expressions to its name, a Symbol.


		""" fresh!(roe::Roe, sort::AbstractSort, name::Symbol)::Var{sort}

		Creates a new ("fresh") variable in a Roe given a sort and a name.

		import Base: +, -, *

		# These operations create calls on a common egraph. We validate the signature by dispatching the operation on the types using methods we defined in Signature.

		export derivative_cost


		""" vfield :: (Decaroe -> (StateVars, ParamVars)) -> VectorFieldFunction

Conversation

quffaro commented Jul 23, 2024

Uh oh!

olynch left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jpfairbanks commented Jul 29, 2024

Uh oh!

KevinDCarlson commented Jul 29, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukem12345 Aug 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lukem12345 Aug 5, 2024 •

edited

Loading

codecov Bot commented Mar 31, 2026 •

edited

Loading