Repository navigation
Give generated symbols deterministic names - #180
Merged
ChrisRackauckas merged 1 commit intoSep 7, 2026
Merged
ChrisRackauckas merged 1 commit into
ChrisRackauckas merged 1 commit into
Conversation
gensym gave every reduced model fresh symbol names, so two reductions of the same system produced different expressions and never shared generated code. Build names from a fixed base plus an obscure suffix instead, and append a counter when the source system already uses the name, which is the case when reducing an already-reduced model. Closes SciML#28. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Agent-Harness: Claude Code 2.1.263 Agent-Model: claude-opus-5 Agent-Session: https://claude.ai/code/session_01H5RuLDtEazwTKPkNbPeZVu Claude-Session: https://claude.ai/code/session_01H5RuLDtEazwTKPkNbPeZVu
ChrisRackauckas
marked this pull request as ready for review
September 7, 2026 05:41
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
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.
Ignore this PR until reviewed by @ChrisRackauckas.
Closes #28.
deimnamed the reduced state and its array parameters withgensym, so every reduction produced fresh names. Two reductions of the same system therefore built different symbolic expressions, which meant differentRuntimeGeneratedFunctions and a full recompile of the problem, the residual, and the reconstruction each time. Names are now built from a fixed base plusGENERATED_SUFFIX, which isˍmor— theˍis U+02CD, the modifier letter ModelingToolkit already uses for generated names such asxˍt, so it is outside the space of names a user would type.Because the names are now shared rather than unique, the source system's own names have to be avoided.
_reserved_namescollects the names the reduced system inherits (kept parameters, source field variables, preserved observed left-hand sides) and_generated_nameappends a counter when the base is taken. That case is reached by reducing an already-reduced model, whose parameters are namedAˍmor,Cˍmorand so on.Failing before, passing after
Reducing the same system twice, on
mainat 2439bac:Same script on this branch:
The new
deterministic generated namestestset intest/array_structure.jlasserts the equal-equations and equal-parameters property, which is what makes the generated code shared; thegenerated names avoid source namestestset covers the counter and a source system that already usesŷˍmor.Measured effect
Three reductions of the same 24-state system in one session, reporting Julia's
@timedcompile-time counter for problem construction, the first residual evaluation, and the first field reconstruction:On
mainthe per-reduction cost never amortizes: ROM 3 still pays the same 0.082 s and 0.041 s as ROM 2. On this branch it goes to zero, because the second reduction reuses the first one's compiled functions. The first reduction is unchanged; it is doing the compilation that the later ones now reuse.Verification
Julia 1.12.7, ModelingToolkit 11.41.0, MethodOfLines 1.3.0.
GROUP=Core julia +1 --project -e 'using Pkg; Pkg.test()'GROUP=QA julia +1 --project -e 'using Pkg; Pkg.test()'julia +1 --project=@runic -m Runic --check .,typos ., andgit diff --checkall exit 0. The QA and docs runs above were made against the identical tree before the final rebase onto the mergedmain; the Core run is post-rebase.Not verified
Being precompiled by another process, and 233 dependencies loaded at different versions), not to this change: with warm caches every test file returns to its usual time, anddeimitself is 0.01 s per call after the first. Worth knowing if CI ever looks slow for an unrelated reason.Review notes
ŷˍmor. They are separate self-contained systems so nothing aliases across them, and the test suite exercises reduced models with POD dimensions 2, 3 and 4 in one session. Flagging it because it is the behavioral change a reviewer would most reasonably question.GENERATED_SUFFIXis not exported and is not documented as public API; the tests reach into it so that they do not hard-code the spelling.🤖 Generated with Claude Code
https://claude.ai/code/session_01H5RuLDtEazwTKPkNbPeZVu