Fix TypeError from nothing-holes in MTKParameters substitution (#4607)#4617
Open
baggepinnen wants to merge 4 commits into
Open
Fix TypeError from nothing-holes in MTKParameters substitution (#4607)#4617baggepinnen wants to merge 4 commits into
baggepinnen wants to merge 4 commits into
Conversation
Partially specifying an array variable (e.g. pinning only one element)
leaves `COMMON_NOTHING` holes for the remaining elements in the
operating point. `MTKParameters` substituted against the raw operating
point, so expressions referencing such hole elements had
`Const(nothing)` folded into arithmetic, throwing
`TypeError: in typeassert, expected DataType, got Type{Union{Nothing, Real}}`
from `promote_symtype` (or `MethodError: *(::Int, ::Nothing)` when all
other arguments are constant). Substitute through
`AtomicArrayDictSubstitutionWrapper` instead, like all other
substitution sites against operating points, so holes are left
symbolic.
Values of `Initial` parameters that consequently cannot be fully
evaluated refer to variables not fixed by the operating point; treat
them (elementwise) as unfixed instead of erroring, generalizing the
existing `val == args[1]` special case. Non-`Initial` parameters retain
the informative "Could not evaluate value of parameter" error.
Fixes #4607
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The recursive traversal visited shared subexpressions repeatedly, making `check_operator_variables` (called per-equation from `generate_rhs`) exponential in the depth of DAG-shaped expressions with heavy sharing. Large torn initialization systems (e.g. multibody models) spun indefinitely here. Track visited subexpressions in a set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The branch called `stable_eachindex` on a plain `Vector{Int}` (no such
method) and referenced an undefined variable `var`. Compute the hashed
guess per missing unknown, mirroring the semantics of the equivalent
branch in `problem_utils.jl`.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Benchmark Results (Julia vlts)Time benchmarks
Memory benchmarks
|
Contributor
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
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.
Fixes #4607.
Root cause
The issue's stacktrace (
promote_symtypefailing a::DataTypetypeassert withUnion{Nothing, Real}) is not caused by a variable with that symtype. Partially specifying an array variable (e.g. pinning a single component of a 3-vector connector flow) leavesCOMMON_NOTHING = Const(nothing)holes for the remaining elements in the operating point (as_atomic_dict_with_defaults/write_possibly_indexed_array!);build_operating_pointonly filters values that are entirelynothing. When another operating-point entry's value expression references such a hole element,MTKParameters— which substituted against the raw operating point — foldedConst(nothing)into arithmetic:promote_symtype(*, Real, Nothing)→promote_type = Union{Nothing, Real}→ typeassert failure (orMethodError: *(::Int, ::Nothing)when all co-arguments are constant).This only fires on the fully-determined initialization path because that is the one that builds an
SCCNonlinearProblemfor the initialization system, whoseprocess_SciMLProblem→MTKParameters(initsys, op)evaluates all parameters (includingInitials) against an op containing the partially-pinned arrays. The underdetermined sibling path instead hits #4237.Changes
lib/ModelingToolkitBase/src/systems/parameter_buffer.jl: substitute throughAtomicArrayDictSubstitutionWrapper(which exists precisely to keep holes symbolic, and is already used by every other substitution site against operating points —problem_utils.jl:348,problem_utils.jl:1599,sccnonlinearproblem.jl:564). Values ofInitialparameters that consequently cannot be fully evaluated refer to variables not fixed by the operating point and are treated (elementwise) as unfixed, generalizing the existingval == args[1] → falsespecial case; this matches howtimevaring_initsys_process_op!turns non-constant pins into initialization equations rather thanInitialvalues. Non-Initialparameters retain the informativeCould not evaluate value of parametererror — which now also replaces the crypticTypeErrorfor genuinely missing values.lib/ModelingToolkitBase/src/utils.jl: memoize the_check_operator_variablestraversal. With the crash fixed, the repro proceeded into per-SCCgenerate_rhsand spun indefinitely (100% CPU, flat memory) in this check, which recursed into shared subexpressions repeatedly — exponential on the DAG-shaped expressions of large torn initialization systems.src/problems/sccnonlinearproblem.jl: theMissingGuessValue.HashedRandom()branch (previously dead code, now reachable) calledstable_eachindexon a plainVector{Int}and referenced an undefined variablevar. Fixed mirroring the equivalent branch inproblem_utils.jl.Validation
lib/ModelingToolkitBase/test/mtkparameters.jl(the two crash modes now produce the informative error; unresolvableInitialvalues build; values referencing specified elements still fold).mtkparameters.jl,initial_values.jl,initializationsystem.jl(ModelingToolkitBase) andscc_nonlinear_problem.jl(ModelingToolkit) test files pass locally.FullCarrepro (DyadMultibodyComponents#suspension, private — not CI-able): pinning the chassis state (r_0,v_0,phi,phid) plus the four redundant chassis reaction torques (excited_suspension_*.chassis_frame.tau[2] => 0) previously threw the exactTypeError; with this PR theODEProblembuilds successfully.solvesubsequently hits aSingularExceptionfrom the chassis static indeterminacy — that is the separate, pre-existing Unhelpful initialization error #4237 and out of scope here.🤖 Generated with Claude Code