[CI smoke, do not merge] PR1 L0 trivial homotopy rewrite#1
Closed
BatchZero wants to merge 29 commits into
Closed
Conversation
This shaves off around 33% of runtime in `mtkcompile` for the model I am testing it on: Before: ``` 11.193316 seconds (136.79 M allocations: 6.120 GiB, 6.71% gc time) ``` After: ``` 7.570217 seconds (85.88 M allocations: 3.767 GiB, 4.58% gc time) ```
refactor: use `IRInfo` and `IRStructure` for `full_equations`
avoid calling `unhack_system` twice for the same `system`
…ructor` This used to be the lion's share of the compile time in large models. The infrastructure added here might be useful in other places too.
Enables supporting ReverseDiff AD
fix: avoid generating massive functions in `get_mtkparameters_reconstructor`
build: bump MTKBase, MTK patch versions
kc/typos whitespace
Implements Modelica spec 3.7.4 trivial form: homotopy(a, s) is a registered symbolic primitive that rewrites to actual=a. Hand-written TermInterface walk (not @rule) per SciML#1385 feedback. Metadata + symtype propagated on rebuilt nodes. Runtime fallback methods for Real and AbstractArray inputs.
Per code review:
- Drop top-level 'using Symbolics' / 'using SymbolicUtils' inside the
included file — MTKBase parent module already imports iscall, operation,
arguments, maketerm, metadata bare (L21-23 of ModelingToolkitBase.jl) and
re-exports Symbolics, so qualified prefixes are redundant noise versus
surrounding files (utils.jl, codegen_utils.jl, if_lifting.jl, etc.).
- Drop unreachable AbstractArray runtime fallback (@register_symbolic is
scalar-only; users go through broadcast).
- Drop arity ArgumentError guard in _rewrite_trivial (@register_symbolic
enforces 2-arity at parse time; guard is dead code).
- Soften docstring: metadata is preserved explicitly via maketerm arg;
symtype is inferred by maketerm itself, not preserved by us.
- Replace brittle !occursin("homotopy", repr(rewritten)) assertion with
!has_homotopy(rewritten), which uses the public predicate and gives
has_homotopy test coverage as a bonus.
Adds has_homotopy_in_equations and rewrite_trivial_in_equations helpers to homotopy.jl (internal use; not exported). Adds RED integration test asserting that an InitializationProblem built from a NonlinearSystem containing homotopy(x^2 - p, x - 1) = 0 solves the actual branch (x^2 = p) rather than the simplified branch (x = 1). Currently fails because the init pipeline does not yet apply rewrite_trivial.
The previous NonlinearSystem variant never exercised the rewrite hook because generate_initializesystem_timeindependent does not migrate the parent system's algebraic equations into the initialization system, so the failure mode was an empty u0 unrelated to homotopy. The new design uses an ODESystem with an algebraic constraint `0 ~ homotopy(y^2 - p, y - 1)` and a free init guess for `y`. This routes through generate_initializesystem_timevarying, which preserves the homotopy-bearing equation in the init system. The structural assertion `!has_homotopy_in_equations(equations(prob.f.sys))` distinguishes hooked from unhooked: without the rewrite, the init system carries the opaque homotopy() symbolic call even though runtime dispatch happens to give a numerically correct answer via the homotopy(::Real, ::Real) fallback.
Detects homotopy() in initialization-system equations and applies rewrite_trivial. Gated by has_homotopy_in_equations so non-homotopy systems pay zero traversal cost. Solves the L0 trivial requirement of Modelica spec 3.7.4. The hook lands before mtkcompile(isys) so that downstream structural transforms operate on the actual equations rather than opaque homotopy() calls; future PRs that lower into a parameter sweep will replace this branch when a HomotopySweep algorithm is in use.
The new homotopy_init.jl covers rewrite_trivial (unit) plus an ODESystem InitializationProblem integration check. It belongs alongside the other Initialization-group entries (guess_propagation, initializationsystem, initial_values) rather than the broader InterfaceI bucket.
Mirrors the structure of Buildings/Fluid/FixedResistances/PressureDrop.mo (from_dp=true branch): nonlinear actual (sqrt-based turbulent law) + linear simplified (nominal-point scaling). PR1 asserts (a) hook fired (no residual homotopy in init equations), (b) init converged, (c) m_flow numerically matches the actual root sqrt(5), not the simplified root m_flow_nominal*dp/dp_nominal = 1.0. Numerical OMC parity deferred to PR2.
Owner
Author
|
Closing — work continues in the same feature/homotopy-operator branch as L1 sweep is accumulated. A single merged L0+L1 PR will be opened upstream to SciML/ModelingToolkit.jl once complete. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Internal CI smoke run — DO NOT MERGE.
Purpose: trigger BatchZero's GitHub Actions on the PR1 implementation
(homotopy operator L0 trivial rewrite for ModelingToolkit.jl) before
considering an upstream PR to SciML/ModelingToolkit.jl.
Upstream PR description is still being reviewed locally and will be
attached to the real PR when it's ready.
Branch: feature/homotopy-operator (11 commits vs upstream/master @ c20dbc6).
Base: BatchZero/master (46f2ef2 — older than upstream; CI here checks the
total diff including upstream commits SciML has merged since 46f2ef2, plus
our PR1 changes).
Local verification done before opening this PR:
This PR exists only to exercise the 48-job CI matrix. Once results are in,
this PR will be closed (or kept as a staging area while iterating).