Roe refactor #62
Roe refactor #62
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #62 +/- ##
=======================================
Coverage 86.80% 86.80%
=======================================
Files 13 13
Lines 894 894
=======================================
Hits 776 776
Misses 118 118 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
This is coming along nicely. It looks like for the Symbolics version, the big feature that we are missing is an analogue of extraction. It would be good if we could reuse the built in compiler for SymbolicUtils. https://github.com/JuliaSymbolics/SymbolicUtils.jl/blob/master/src/code.jl
Specifically, we should be able to use toexpr to generate functions that evaluate the tangent variables. https://github.com/JuliaSymbolics/SymbolicUtils.jl/blob/2ef075e4274746d9c4b6c3215f385867b0c85edf/src/code.jl#L348
| """ | ||
| function create_dynamic_model(sd, hodge)::Dict{TypedApplication, Any} | ||
| Dict{TypedApplication, Any}( | ||
| TA(*, Sort[Scalar(), Scalar()]) => 1, |
There was a problem hiding this comment.
what if we add functions for op(sorts::VarArg{Sort}) = TA(op, sorts) for each operation in the theory so that this could be *(Scalar(), Scalar()) => 1
| @nospecialize | ||
| function d(s::Sort) | ||
| @match s begin | ||
| Scalar() => throw(SortError("Cannot take exterior derivative of a scalar")) |
There was a problem hiding this comment.
Should this be the 0 scalar? Because the derivative of a constant is 0? Scalars often get implicitly converted to the constant Form with that value. d(x+1) should be equal to d(x) + d(1), which implies d(1) = 0
| # Δ = ★d⋆d, but we check signature here to throw a more helpful error | ||
| function Δ(s::Sort) | ||
| @match s begin | ||
| Form(0, isdual) => Form(0, isdual) |
There was a problem hiding this comment.
There is a 1D Laplacian too
| @eval begin | ||
| @nospecialize | ||
| function $unop( | ||
| v::SymbolicUtils.BasicSymbolic{OfSort{s}} |
There was a problem hiding this comment.
Why are we wrapping the Sort in another type OfSort here?
| v::SymbolicUtils.BasicSymbolic{OfSort{s1}}, | ||
| w::SymbolicUtils.BasicSymbolic{OfSort{s2}} | ||
| ) where {s1,s2} | ||
| s′ = $binop(s1, s2) |
There was a problem hiding this comment.
SymbolicUtils uses promote_symtype on the types rather than having the operation act directly on the types.
| # but I felt this was visually too noisy in `build_result_body`. | ||
| function toexpr(expr::Union{Term, DEC.SSAs.Var}, op_lookup, rv_lookup) | ||
| λtoexpr = @λ begin | ||
| var::DEC.SSAs.Var => Symbol("tmp%$(var.idx)") |
There was a problem hiding this comment.
we have been using \bullet in the existing compiler. It would be good to match for visual consistency between versions.
| (rv, false) => rv | ||
| (rv, true) => Expr(:ref, rv, term.head.name) | ||
| end | ||
| # This default case is Decapodes-specific. Decapode operators return a tuple of functions, so we choose the first. |
There was a problem hiding this comment.
I think this is dependent on using the preallocate flag. It would be good to be able to extract either in-place or out-of-place if possible. Although out-of-place is actually higher priority right now because of autodiff constraints.
|
|
||
| ```julia | ||
| function klausmeier(roe::Roe) | ||
| @vars roe n::DualForm0 w::DualForm0 dX::Form1 a::Constant{DualForm0} ν::Constant{DualForm0} |
There was a problem hiding this comment.
Should these be replaced by n::Form(0,Dual) or something? or does @vars handle legacy decapodes names and replace them?
| klausmeier(namespaced(roe, :k1)) | ||
| klausmeier(namespaced(roe, :k2)) | ||
|
|
||
| @eq roe (roe.k1.m == roe.k2.m) |
There was a problem hiding this comment.
having them share the same dX and w would make sense because that would give you two species of plant sharing the same water supply. Although at thought point, you would want to break the models down into smaller pieces and reassemble it with only one water supply.
This is how you would it in current Decapodes. You have to break out any factor that gets superposition into its own models so that colimits the category of diagrams do the right thing.
# See Klausmeier Equation 2.a
Hydrodynamics = @decapode begin
(w, consumption)::DualForm0
dX::Form1
(a,ν)::Constant
∂ₜ(w) == a - w - consumption + ν * L(dX, w)
end
# See Klausmeier Equation 2.b
Phytodynamics = @decapode begin
(n,w)::DualForm0
m::Constant
consumption = w * n^2
∂ₜ(n) == consumption - m*n + Δ(n)
end
Superposition = @decapde begin
total = part1 + part2
end
compose_klausmeier = @relation () begin
phyto(N1, W, consumption1)
phyto(N2, W, consumption2)
superposition(consumption, consumption1, consumption2)
hydro(consumption, W)
end
klausmeier_cospan = oapply(compose_klausmeier,
[Open(Phytodynamics, [:consumption, :w]), Open(Phytodynamics, [:consumption, :w])
Open(Hydrodynamics, [:total, :part1, :part2]),
Open(Hydrodynamics, [:consumption, :w])])
Klausmeier = apex(klausmeier_cospan)I guess you could do it in Roe by replacing the definition of a term with a sum? That seems like it would cause you to flush the egraph and resaturate, because you removed an equation.
| struct OfSort{s} <: Number end | ||
| export OfSort | ||
|
|
||
| struct SortMetadata end |
There was a problem hiding this comment.
Was this for using metadata for the types, but actually unnecessary because you figured out how to use SymbolicUtils.Sym{OfSort{sort}}(name) to put the actual types in there?
No description provided.