Skip to content

fix: pin the unassigned type parameter of the observed dependency graph - #4887

Merged
AayushSabharwal merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:fix/observed-graph-cache-matching-type
Aug 6, 2026
Merged

fix: pin the unassigned type parameter of the observed dependency graph#4887
AayushSabharwal merged 1 commit into
SciML:masterfrom
ChrisRackauckas-Claude:fix/observed-graph-cache-matching-type

Conversation

@ChrisRackauckas-Claude

Copy link
Copy Markdown
Member

This PR should be ignored until reviewed by @ChrisRackauckas.

The failure

On unmodified master, GROUP=InterfaceI errors:

Non-`Real` symtype parameters in callback with unknown: Error During Test at
  lib/ModelingToolkitBase/test/symbolic_events.jl:1643
  Got exception outside of a @test
  MethodError: Cannot `convert` an object of type
    DiCMOBiGraph{false,Int64,BipartiteGraph{Int64,Nothing},Matching{Union{Unassigned, Int64},Array{Union{Unassigned, Int64},1}}}
  to an object of type
    DiCMOBiGraph{false,Int64,BipartiteGraph{Int64,Nothing},Matching{Unassigned,Array{Union{Unassigned, Int64},1}}}
  Stacktrace:
   [1] ModelingToolkitBase.ObservedGraphCache(...)
     @ ModelingToolkitBase lib/ModelingToolkitBase/src/utils.jl:1222
   [2] observed_equations_used_by(...)
     @ ModelingToolkitBase lib/ModelingToolkitBase/src/utils.jl:1268
   [3] generate_rhs(...)
     @ ModelingToolkitBase lib/ModelingToolkitBase/src/systems/codegen.jl:73

Root cause

observed_dependency_graph builds its matching as

matching = complete(Matching(Vector{Union{Unassigned, Int}}(assigns)))

and relies on Matching(v) inferring Unassigned as the unassigned type parameter, because ObservedGraphCache declares its graph field with the fully concrete type DiCMOBiGraph{false, Int, BipartiteGraph{Int, Nothing}, Matching{Unassigned, Vector{Union{Unassigned, Int}}}}.

BipartiteGraphs v0.1.11 (registered 2026-08-04) changed that inference. Verified locally on two pinned environments:

complete(Matching(Vector{Union{Unassigned, Int}}([1, 2, 3]))) |> typeof
# v0.1.10 -> Matching{Unassigned, Vector{Union{Unassigned, Int64}}}
# v0.1.11 -> Matching{Union{Unassigned, Int64}, Vector{Union{Unassigned, Int64}}}

The change comes from SciML/BipartiteGraphs.jl@e5bc591, which replaced

function Matching(v::V) where {U, V <: AbstractVector{Union{U, Int}}}
    return Matching{@isdefined(U) ? U : Unassigned, V}(v, nothing)
end

with a version that uses the whole eltype as U. There is no convert between the two DiCMOBiGraph types, so populating ObservedGraphCache throws. Nothing in ModelingToolkit changed — the repo's Manifest.toml is untracked, so the regression appeared purely from the dependency resolving to v0.1.11. Tracked upstream as SciML/BipartiteGraphs.jl#54.

The fix

Construct Matching{Unassigned} explicitly instead of depending on inference that is not part of a stable contract, and share the concrete graph type between observed_dependency_graph and the ObservedGraphCache field so the two cannot drift apart again. This works against both v0.1.10 and v0.1.11, so no compat bound change is needed.

Reproduction and verification

Minimal reproduction of the underlying bug (fails on master, passes here):

using ModelingToolkit
using ModelingToolkit: t_nounits as t, D_nounits as D
import ModelingToolkitBase as MTKB
@variables x(t) y(t)
@parameters p
@mtkcompile sys = System([D(x) ~ -x + y, y ~ 2x + p], t)
MTKB.observed_equations_used_by(sys, [equations(sys)[1].rhs])

Runs performed locally (Julia 1.11.9, BipartiteGraphs v0.1.11):

  • lib/ModelingToolkitBase/test/symbolic_events.jl without this patch — errors at the Non-Real symtype parameters in callback with unknown testset, exactly as CI does.
  • lib/ModelingToolkitBase/test/symbolic_events.jl with this patch — whole file passes, including that testset.
  • lib/ModelingToolkitBase/test/code_generation.jl with this patch — passes, including the new regression testset.

Note that code_generation.jl (group InterfaceII) hits the same code path, so that group is affected too.

Regression test

code_generation.jl gets a testset asserting that observed_dependency_graph's result is assignable to the ObservedGraphCache graph field and that populating the cache via observed_equations_used_by does not throw.

`observed_dependency_graph` relied on `Matching(v)` inferring `Unassigned`
as the unassigned type parameter from a `Vector{Union{Unassigned, Int}}`.
BipartiteGraphs v0.1.11 changed that inference to yield
`Matching{Union{Unassigned, Int}}`, which no longer matches the concrete
field type of `ObservedGraphCache` and makes populating the cache throw a
`convert` `MethodError`.

Construct `Matching{Unassigned}` explicitly and share the concrete graph
type between the constructor and the cache field so they cannot drift.

Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com>
@AayushSabharwal

Copy link
Copy Markdown
Member

Why is the fix here? BipartiteGraphs should revert the breaking change.

@AayushSabharwal

Copy link
Copy Markdown
Member

The changes aren't technically incorrect and while I'm not particularly sold on the BipartiteGraphs change this PR might as well merge so CI is functional again.

@AayushSabharwal
AayushSabharwal marked this pull request as ready for review August 6, 2026 08:33
@AayushSabharwal
AayushSabharwal merged commit 54e1fe6 into SciML:master Aug 6, 2026
82 of 93 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants