Skip to content

Commit d9e1b2b

Browse files
ChrisRackauckas-ClaudeChrisRackauckas
ChrisRackauckas-Claude
and
ChrisRackauckas
authored
Fix collect_vars! after late invalidation on Julia 1.10 (#4852)
* Fix collect_vars! after late invalidation Route recursive metadata collection through a dynamic dispatch barrier so late method additions cannot corrupt Julia 1.10 inference while downstream collect_vars! extensions remain visible. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> * Use @invokelatest for the collect_vars! dispatch barrier Replaces the global Ref{Function} cell with @invokelatest, per review. Both break the Julia 1.10 inference cycle identically and cost the same, but @invokelatest also resolves in the latest world age, so a downstream collect_vars! method defined while a collection is already running is still found. The four-argument fallback returns nothing, so missing such a method dropped parameters silently rather than erroring. The @noinline and Base.@constprop :none annotations are no longer needed, since @invokelatest is already opaque to inference. Co-Authored-By: Chris Rackauckas <accounts@chrisrackauckas.com> --------- Co-authored-by: ChrisRackauckas-Claude <accounts@chrisrackauckas.com>
1 parent b3ff68b commit d9e1b2b

2 files changed

Lines changed: 115 additions & 44 deletions

File tree

‎lib/ModelingToolkitBase/src/utils.jl‎

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -936,6 +936,16 @@ function collect_vars!(
936936
return nothing
937937
end
938938

939+
# Break the inference cycle between the mutually recursive collectors, which Julia
940+
# 1.10 can otherwise miscompile after unrelated method additions. Dispatching in the
941+
# latest world age also means a downstream `collect_vars!` method defined while this
942+
# call is already running is still found; because the four-argument fallback returns
943+
# `nothing`, missing it would silently drop parameters rather than error. The explicit
944+
# keyword preserves metadata recursion's depth-zero semantics.
945+
function _call_collect_vars!(unknowns, parameters, expr, iv)
946+
return @invokelatest collect_vars!(unknowns, parameters, expr, iv; depth = 0)
947+
end
948+
939949
"""
940950
$(TYPEDSIGNATURES)
941951
@@ -966,7 +976,7 @@ function collect_var!(unknowns::OrderedSet{SymbolicT}, parameters::OrderedSet{Sy
966976
any(!SU.isconst, Iterators.drop(arguments(var), 1))
967977
)
968978
for arg in Iterators.drop(arguments(var), 1)
969-
collect_vars!(unknowns, parameters, arg, iv)
979+
_call_collect_vars!(unknowns, parameters, arg, iv)
970980
end
971981
var = arr
972982
end
@@ -975,7 +985,7 @@ function collect_var!(unknowns::OrderedSet{SymbolicT}, parameters::OrderedSet{Sy
975985
if iscalledparameter(var)
976986
callable = getcalledparameter(var)
977987
push!(parameters, callable)
978-
collect_vars!(unknowns, parameters, arguments(var), iv)
988+
_call_collect_vars!(unknowns, parameters, arguments(var), iv)
979989
elseif isparameter(var) || (iscall(var) && isparameter(operation(var)))
980990
push!(parameters, var)
981991
else
@@ -984,55 +994,55 @@ function collect_var!(unknowns::OrderedSet{SymbolicT}, parameters::OrderedSet{Sy
984994
# Add also any parameters that appear only as defaults in the var
985995
if hasdefault(var) && (def = getdefault(var)) !== missing
986996
if def isa SymbolicT
987-
collect_vars!(unknowns, parameters, def, iv)
997+
_call_collect_vars!(unknowns, parameters, def, iv)
988998
elseif def isa Num
989-
collect_vars!(unknowns, parameters, def, iv)
999+
_call_collect_vars!(unknowns, parameters, def, iv)
9901000
elseif def isa Arr{Num, 1}
991-
collect_vars!(unknowns, parameters, def, iv)
1001+
_call_collect_vars!(unknowns, parameters, def, iv)
9921002
elseif def isa Arr{Num, 2}
993-
collect_vars!(unknowns, parameters, def, iv)
1003+
_call_collect_vars!(unknowns, parameters, def, iv)
9941004
elseif def isa CallAndWrap{Num}
995-
collect_vars!(unknowns, parameters, def, iv)
1005+
_call_collect_vars!(unknowns, parameters, def, iv)
9961006
elseif def isa CallAndWrap{Arr{Num, 1}}
997-
collect_vars!(unknowns, parameters, def, iv)
1007+
_call_collect_vars!(unknowns, parameters, def, iv)
9981008
elseif def isa CallAndWrap{Arr{Num, 2}}
999-
collect_vars!(unknowns, parameters, def, iv)
1009+
_call_collect_vars!(unknowns, parameters, def, iv)
10001010
elseif def isa Arr
1001-
collect_vars!(unknowns, parameters, def, iv)
1011+
_call_collect_vars!(unknowns, parameters, def, iv)
10021012
elseif def isa CallAndWrap
1003-
collect_vars!(unknowns, parameters, def, iv)
1013+
_call_collect_vars!(unknowns, parameters, def, iv)
10041014
else
1005-
collect_vars!(unknowns, parameters, def, iv)
1015+
_call_collect_vars!(unknowns, parameters, def, iv)
10061016
end
10071017
end
10081018
# Add also any parameters that appear only in the bounds of the var
10091019
if hasbounds(var)
10101020
(lo, hi) = getbounds(var)
10111021
if lo isa SymbolicT
1012-
collect_vars!(unknowns, parameters, lo, iv)
1022+
_call_collect_vars!(unknowns, parameters, lo, iv)
10131023
elseif lo isa Num
1014-
collect_vars!(unknowns, parameters, lo, iv)
1024+
_call_collect_vars!(unknowns, parameters, lo, iv)
10151025
elseif lo isa Arr{Num, 1}
1016-
collect_vars!(unknowns, parameters, lo, iv)
1026+
_call_collect_vars!(unknowns, parameters, lo, iv)
10171027
elseif lo isa Arr{Num, 2}
1018-
collect_vars!(unknowns, parameters, lo, iv)
1028+
_call_collect_vars!(unknowns, parameters, lo, iv)
10191029
elseif lo isa Arr
1020-
collect_vars!(unknowns, parameters, lo, iv)
1030+
_call_collect_vars!(unknowns, parameters, lo, iv)
10211031
else
1022-
collect_vars!(unknowns, parameters, lo, iv)
1032+
_call_collect_vars!(unknowns, parameters, lo, iv)
10231033
end
10241034
if hi isa SymbolicT
1025-
collect_vars!(unknowns, parameters, hi, iv)
1035+
_call_collect_vars!(unknowns, parameters, hi, iv)
10261036
elseif hi isa Num
1027-
collect_vars!(unknowns, parameters, hi, iv)
1037+
_call_collect_vars!(unknowns, parameters, hi, iv)
10281038
elseif hi isa Arr{Num, 1}
1029-
collect_vars!(unknowns, parameters, hi, iv)
1039+
_call_collect_vars!(unknowns, parameters, hi, iv)
10301040
elseif hi isa Arr{Num, 2}
1031-
collect_vars!(unknowns, parameters, hi, iv)
1041+
_call_collect_vars!(unknowns, parameters, hi, iv)
10321042
elseif hi isa Arr
1033-
collect_vars!(unknowns, parameters, hi, iv)
1043+
_call_collect_vars!(unknowns, parameters, hi, iv)
10341044
else
1035-
collect_vars!(unknowns, parameters, hi, iv)
1045+
_call_collect_vars!(unknowns, parameters, hi, iv)
10361046
end
10371047
end
10381048
return nothing

‎lib/ModelingToolkitBase/test/dq_units.jl‎

Lines changed: 81 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ using ModelingToolkitBase, OrdinaryDiffEq, JumpProcesses, DynamicQuantities
22
using Symbolics
33
import SymbolicUtils as SU
44
using Test
5+
using DataStructures: OrderedSet
56
MT = ModelingToolkitBase
67
using ModelingToolkitBase: t, D
78
@parameters τ [unit = u"s"] γ
@@ -273,30 +274,90 @@ let
273274
@test MT.get_unit(x_mat) == u"1"
274275
end
275276

276-
# Issue #4211: collect_vars! must discover parameters in defaults with DynamicQuantities loaded
277-
# On Julia 1.10, loading DynamicQuantities could cause collect_vars! to fail to discover
278-
# parameters used in variable defaults due to a method invalidation bug.
279-
@testset "Issue #4211: collect_vars! discovers parameters in defaults" begin
280-
using DataStructures: OrderedSet
277+
struct CollectorInvalidationString
278+
value::String
279+
end
280+
281+
struct CollectorInvalidationExpression
282+
parameter::Symbolics.SymbolicT
283+
end
284+
285+
struct CollectorSameWorldExpression
286+
parameter::Symbolics.SymbolicT
287+
end
281288

282-
@parameters X0_test
283-
@variables X_test(t) = X0_test
289+
function collect_default_parameters(var)
290+
unknowns = OrderedSet{Symbolics.SymbolicT}()
291+
parameters = OrderedSet{Symbolics.SymbolicT}()
292+
MT.collect_vars!(
293+
unknowns, parameters, Symbolics.unwrap(var), Symbolics.unwrap(t),
294+
Symbolics.Operator; depth = 0
295+
)
296+
return parameters
297+
end
298+
299+
@testset "collect_vars! survives late method invalidation" begin
300+
@parameters x0_test y0_test scale_test
301+
@variables x_test(t) = x0_test y_test(t) = scale_test * y0_test
284302

285-
us = OrderedSet{Symbolics.SymbolicT}()
286-
ps = OrderedSet{Symbolics.SymbolicT}()
287-
MT.collect_vars!(us, ps, Symbolics.unwrap(X_test), Symbolics.unwrap(t), Symbolics.Operator; depth = 0)
303+
@test Symbolics.unwrap(x0_test) in collect_default_parameters(x_test)
288304

289-
# X0_test should be discovered in X_test's default value
290-
@test Symbolics.unwrap(X0_test) in ps
305+
# This late definition exercises the Julia 1.10 invalidation that caused
306+
# parameters in defaults to disappear after loading unrelated packages.
307+
@eval Base.convert(::Type{Symbol}, value::CollectorInvalidationString) =
308+
Symbol(value.value)
291309

292-
# Test with expression in default
293-
@parameters a_test b_test
294-
@variables Y_test(t) = a_test + 2 * b_test
310+
x_parameters = collect_default_parameters(x_test)
311+
y_parameters = collect_default_parameters(y_test)
295312

296-
empty!(us)
297-
empty!(ps)
298-
MT.collect_vars!(us, ps, Symbolics.unwrap(Y_test), Symbolics.unwrap(t), Symbolics.Operator; depth = 0)
313+
@test Symbolics.unwrap(x0_test) in x_parameters
314+
@test Symbolics.unwrap(y0_test) in y_parameters
315+
@test Symbolics.unwrap(scale_test) in y_parameters
316+
317+
custom_default = MT.setdefault(
318+
x_test, CollectorInvalidationExpression(Symbolics.unwrap(scale_test))
319+
)
320+
@eval function MT.collect_vars!(
321+
unknowns::OrderedSet{Symbolics.SymbolicT},
322+
parameters::OrderedSet{Symbolics.SymbolicT},
323+
expr::CollectorInvalidationExpression,
324+
::Union{Symbolics.SymbolicT, Nothing};
325+
depth = 0
326+
)
327+
push!(parameters, expr.parameter)
328+
return nothing
329+
end
330+
331+
@test Symbolics.unwrap(scale_test) in collect_default_parameters(custom_default)
332+
@test Symbolics.unwrap(x0_test) in collect_default_parameters(x_test)
333+
end
334+
335+
# Defines the downstream method from inside a running call, so the enclosing frame
336+
# keeps its old world age while collecting.
337+
function define_then_collect(var)
338+
@eval function MT.collect_vars!(
339+
unknowns::OrderedSet{Symbolics.SymbolicT},
340+
parameters::OrderedSet{Symbolics.SymbolicT},
341+
expr::CollectorSameWorldExpression,
342+
::Union{Symbolics.SymbolicT, Nothing};
343+
depth = 0
344+
)
345+
push!(parameters, expr.parameter)
346+
return nothing
347+
end
348+
return collect_default_parameters(var)
349+
end
350+
351+
@testset "collect_vars! dispatches to same-world-age downstream methods" begin
352+
@parameters same_world_test
353+
@variables same_world_var(t)
354+
355+
target = MT.setdefault(
356+
same_world_var, CollectorSameWorldExpression(Symbolics.unwrap(same_world_test))
357+
)
299358

300-
@test Symbolics.unwrap(a_test) in ps
301-
@test Symbolics.unwrap(b_test) in ps
359+
# The four-argument fallback returns `nothing`, so failing to see the method here
360+
# would drop the parameter silently instead of raising a `MethodError`.
361+
@test Symbolics.unwrap(same_world_test) in define_then_collect(target)
362+
@test Symbolics.unwrap(same_world_test) in collect_default_parameters(target)
302363
end

0 commit comments

Comments
 (0)