Fix HashedRandom guess in SCCNonlinearProblem (stable_eachindex MethodError)#4606
Open
baggepinnen wants to merge 2 commits into
Open
Fix HashedRandom guess in SCCNonlinearProblem (stable_eachindex MethodError)#4606baggepinnen wants to merge 2 commits into
baggepinnen wants to merge 2 commits into
Conversation
The `MissingGuessValue.HashedRandom()` branch in the SCC init-problem
builder called `stable_eachindex` on `symbolic_idxs`, a plain
`Vector{Int64}` returned by `findall`. `stable_eachindex` is only defined
for `BasicSymbolic`, so this threw `MethodError: no method matching
stable_eachindex(::Vector{Int64})`. The line also referenced `var`, which
is undefined in this branch (it only exists in the `LinearFunction`
branch) — the line was a faulty copy of the array-symbolic code in
`problem_utils.jl`.
For the SCC case the missing-guess value for index `j` should be derived
from the corresponding scalar unknown `dvs[vscc[j]]`, matching the scalar
`HashedRandom` form `hash(var)/0x1p64` and consistent with the sibling
`Random` branch. This branch only became reachable once HashedRandom
became the default missing-guess strategy (SciML#4445).
Fixes SciML#4603
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds a self-contained test where the initialization problem decomposes
into multiple SCCs with no user-provided guesses, exercising the default
`HashedRandom` missing-guess branch in `SCCNonlinearProblem`. Without the
fix this errors with `MethodError: no method matching
stable_eachindex(::Vector{Int64})`.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AayushSabharwal
approved these changes
Jun 9, 2026
Contributor
Author
|
good to merge? This is keeping MBC CI red and dyad-lang wants to add MBC as downstream test |
oscardssmith
approved these changes
Jun 11, 2026
Member
|
CI seems mad at you, but in principle looks good |
Contributor
Author
|
I haven't learned how to interpret MTK CI, it seems about half are red all the time? Are there any actions needed before merge? |
Member
|
very much an @AayushSabharwal question. |
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.
Fixes #4603
The
MissingGuessValue.HashedRandom()branch in the SCC init-problem builder (src/problems/sccnonlinearproblem.jl) had two bugs on one line:symbolic_idxsis a plainVector{Int64}(fromfindall), butstable_eachindexis only defined forBasicSymbolic, producing the reportedMethodError: no method matching stable_eachindex(::Vector{Int64}). This was introduced by a blanketeachindex→stable_eachindexrename that wrongly caught this loop over a regular vector.varis undefined in this (non-LinearFunction) branch — it only exists in theLinearFunctionbranch. The line was a faulty copy of the array-symbolicHashedRandomcode inproblem_utils.jl.For the SCC case, the missing-guess value for index
jshould derive from the corresponding scalar unknowndvs[vscc[j]], matching the scalarHashedRandomformhash(var)/0x1p64and consistent with the siblingRandombranch:This branch only became reachable once
HashedRandombecame the default missing-guess strategy (#4445), which is why it surfaced as a regression.Verification
Reproduced with the
ParallelKinematicRobotmodel from MultibodyComponents.jl (the case in #4603):MethodError: no method matching stable_eachindex(::Vector{Int64})duringODEProblemconstruction.ODEProblembuilds andsolvereturnsretcode: Success.🤖 Generated with Claude Code