From de0891729f19fddc014e00011b12681298d5c1bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9mi=20Vezy?= Date: Sun, 6 Sep 2026 22:12:07 +0200 Subject: [PATCH] Avoid rebuilding unchanged statuses during compilation --- src/composite_model/compilation.jl | 158 +++++++++++++++++++----- test/test-initial-status-preparation.jl | 135 +++++++++++++++++++- 2 files changed, 261 insertions(+), 32 deletions(-) diff --git a/src/composite_model/compilation.jl b/src/composite_model/compilation.jl index 4109f1ed..dab70b6e 100644 --- a/src/composite_model/compilation.jl +++ b/src/composite_model/compilation.jl @@ -5096,7 +5096,7 @@ mutable struct _CanonicalStatusRecipe original::Union{Nothing,Status} names::Vector{Symbol} references::Union{Vector{Base.RefValue},Vector{Ref}} - positions::Dict{Symbol,Int} + positions::Union{Nothing,Dict{Symbol,Int}} changed::Bool field_changes::Int end @@ -5112,10 +5112,15 @@ function _CanonicalStatusRecipe(status::Union{Nothing,Status}) else _status_recipe_references(refvalues(status)) end - positions = Dict{Symbol,Int}() - sizehint!(positions, length(names)) - for (index, name) in pairs(names) - positions[name] = index + positions = if length(names) > 1 + index = Dict{Symbol,Int}() + sizehint!(index, length(names)) + for (position, name) in pairs(names) + index[name] = position + end + index + else + nothing end return _CanonicalStatusRecipe( status, @@ -5127,14 +5132,22 @@ function _CanonicalStatusRecipe(status::Union{Nothing,Status}) ) end +@inline function _status_recipe_position(recipe::_CanonicalStatusRecipe, variable::Symbol) + positions = recipe.positions + isnothing(positions) || return get(positions, variable, 0) + return !isempty(recipe.names) && first(recipe.names) === variable ? 1 : 0 +end + @inline _status_recipe_has_variable(recipe::_CanonicalStatusRecipe, variable::Symbol) = - haskey(recipe.positions, variable) + !iszero(_status_recipe_position(recipe, variable)) function _status_recipe_reference( recipe::_CanonicalStatusRecipe, variable::Symbol, ) - return recipe.references[recipe.positions[variable]] + position = _status_recipe_position(recipe, variable) + iszero(position) && throw(KeyError(variable)) + return recipe.references[position] end function _status_recipe_set_reference!( @@ -5142,11 +5155,19 @@ function _status_recipe_set_reference!( variable::Symbol, reference::Ref, ) - position = get(recipe.positions, variable, 0) + position = _status_recipe_position(recipe, variable) if iszero(position) push!(recipe.names, variable) push!(recipe.references, reference) - recipe.positions[variable] = length(recipe.names) + if isnothing(recipe.positions) + # Zero/one-field recipes need no lookup table. Once a second field + # is added, use the same indexed path as wider status schemas. + if length(recipe.names) == 2 + recipe.positions = Dict(first(recipe.names) => 1, variable => 2) + end + else + recipe.positions[variable] = length(recipe.names) + end elseif recipe.references[position] === reference return false else @@ -5188,6 +5209,18 @@ function _finish_status_recipe(recipe::_CanonicalStatusRecipe) return Status(NamedTuple{names}(references)) end +function _model_object_status_for_preparation( + model::CompositeModel, + object_id::ObjectId, +) + status = _model_object(model, object_id).status + (isnothing(status) || status isa Status) || error( + "Model object `$(object_id.value)` uses model applications but its status has type " * + "`$(typeof(status))`. Use `Status(...)` or leave status as `nothing`." + ) + return status +end + function _status_recipe_for_object!( recipes::Dict{ObjectId,_CanonicalStatusRecipe}, recipe_order::Vector{ObjectId}, @@ -5195,17 +5228,27 @@ function _status_recipe_for_object!( object_id::ObjectId, ) return get!(recipes, object_id) do - object = _model_object(model, object_id) - status = object.status - (isnothing(status) || status isa Status) || error( - "Model object `$(object_id.value)` uses model applications but its status has type " * - "`$(typeof(status))`. Use `Status(...)` or leave status as `nothing`." - ) + status = _model_object_status_for_preparation(model, object_id) push!(recipe_order, object_id) _CanonicalStatusRecipe(status) end end +function _status_preparation_for_object!( + recipes::Dict{ObjectId,_CanonicalStatusRecipe}, + recipe_order::Vector{ObjectId}, + model::CompositeModel, + object_id::ObjectId, +) + recipe = get(recipes, object_id, nothing) + isnothing(recipe) || return recipe + status = _model_object_status_for_preparation(model, object_id) + # Existing fields can be checked without copying their names and references. + # A missing status still needs a staged, distinct Status even with no ports. + isnothing(status) || return status + return _status_recipe_for_object!(recipes, recipe_order, model, object_id) +end + function _apply_status_recipes!( model::CompositeModel, recipes::Dict{ObjectId,_CanonicalStatusRecipe}, @@ -5324,7 +5367,7 @@ function _prepare_model_output_statuses_batched!( for application in applications defaults = outputs_(application.spec) for object_id in application.target_ids - recipe = _status_recipe_for_object!( + preparation = _status_preparation_for_object!( recipes, recipe_order, model, @@ -5333,9 +5376,18 @@ function _prepare_model_output_statuses_batched!( for (variable, value) in pairs(defaults) _publish_mode_for_output(application.spec, variable) == :canonical || continue + if preparation isa Status + variable in propertynames(preparation) && continue + preparation = _status_recipe_for_object!( + recipes, + recipe_order, + model, + object_id, + ) + end _status_recipe_add_default!( model, - recipe, + preparation, object_id, variable, value; @@ -5426,7 +5478,7 @@ function _prepare_model_output_destination_statuses!( try for resolved in resolved_destinations for destination_id in resolved.destination_ids - recipe = _status_recipe_for_object!( + preparation = _status_preparation_for_object!( recipes, recipe_order, model, @@ -5435,9 +5487,18 @@ function _prepare_model_output_destination_statuses!( for (variable_, declaration) in pairs(resolved.plan.declarations) declaration isa Default || continue variable = Symbol(variable_) + if preparation isa Status + variable in propertynames(preparation) && continue + preparation = _status_recipe_for_object!( + recipes, + recipe_order, + model, + destination_id, + ) + end _status_recipe_add_default!( model, - recipe, + preparation, destination_id, variable, _input_default(declaration); @@ -6015,7 +6076,7 @@ function _prepare_model_input_statuses_batched!( schema = _input_schema(application.spec) defaults = _input_default_values(schema) for object_id in application.target_ids - recipe = _status_recipe_for_object!( + preparation = _status_preparation_for_object!( recipes, recipe_order, model, @@ -6023,7 +6084,17 @@ function _prepare_model_input_statuses_batched!( ) for (variable_, value) in pairs(defaults) variable = Symbol(variable_) - _status_recipe_has_variable(recipe, variable) && continue + if preparation isa Status + variable in propertynames(preparation) && continue + preparation = _status_recipe_for_object!( + recipes, + recipe_order, + model, + object_id, + ) + else + _status_recipe_has_variable(preparation, variable) && continue + end reference = isnothing(final_references) ? nothing : get( @@ -6046,7 +6117,7 @@ function _prepare_model_input_statuses_batched!( conversion_records=conversion_records, ) _status_recipe_set_reference!( - recipe, + preparation, variable, reference, ) @@ -6055,7 +6126,7 @@ function _prepare_model_input_statuses_batched!( end _status_recipe_add_default!( model, - recipe, + preparation, object_id, variable, value; @@ -6075,12 +6146,6 @@ function _prepare_model_input_statuses_batched!( if isnothing(recipe) status = _model_object(model, binding.consumer_id).status status isa Status || continue - recipe = _status_recipe_for_object!( - recipes, - recipe_order, - model, - binding.consumer_id, - ) end reference = if isnothing(final_references) _model_input_status_reference(binding) @@ -6097,8 +6162,20 @@ function _prepare_model_input_statuses_batched!( end isnothing(reference) && (reference = _model_input_status_reference(binding)) - if _status_recipe_has_variable(recipe, binding.input) && - _status_recipe_reference(recipe, binding.input) === reference + if isnothing(recipe) + if binding.input in propertynames(status) && + refvalue(status, binding.input) === reference + default_status_updates[(binding.consumer_id, binding.input)] = false + continue + end + recipe = _status_recipe_for_object!( + recipes, + recipe_order, + model, + binding.consumer_id, + ) + elseif _status_recipe_has_variable(recipe, binding.input) && + _status_recipe_reference(recipe, binding.input) === reference default_status_updates[(binding.consumer_id, binding.input)] = false continue end @@ -6265,6 +6342,7 @@ function _validate_temporal_input_output_overlap!( application::CompiledModelApplication, temporal_bindings, ) + isempty(temporal_bindings) && return nothing output_names = Set(Symbol.(keys(outputs_(application.spec)))) for binding in temporal_bindings binding.input in output_names || continue @@ -6294,6 +6372,24 @@ Base.@nospecializeinfer function _compile_model_status_view( # The assembled Status and CompiledModelStatusView still have concrete types. @nospecialize application input_bindings canonical_status = _ensure_model_object_status!(model, object_id) + has_temporal_inputs = false + for binding in input_bindings + if binding.carrier_hint == :temporal_stream + has_temporal_inputs = true + break + end + end + # Ordinary applications use the canonical status directly. Avoid preparing + # temporal/private scratch storage and validating a nonexistent overlap. + if !has_temporal_inputs && isempty(output_routing(application.spec)) + return CompiledModelStatusView( + canonical_status, + canonical_status, + (), + NamedTuple(), + _compiled_bound_many_inputs(input_bindings, canonical_status), + ) + end temporal_bindings = CompiledModelInputBinding[] for binding in input_bindings binding.carrier_hint == :temporal_stream && push!(temporal_bindings, binding) diff --git a/test/test-initial-status-preparation.jl b/test/test-initial-status-preparation.jl index ae9710e5..918a1da6 100644 --- a/test/test-initial-status-preparation.jl +++ b/test/test-initial-status-preparation.jl @@ -44,6 +44,88 @@ initial_preparation_status(model, id) = only( object.status for object in model_objects(model) if object.id == ObjectId(id) ) +@testset "complete initial statuses retain identity and arbitrary Ref aliases" begin + storage = [7.5, 2.25] + supplied_reference = Ref(storage, 1) + result_reference = Ref(storage, 2) + original = Status(( + supplied=supplied_reference, + alias=supplied_reference, + result=result_reference, + offset=Ref(3.0), + )) + model = CompositeModel( + Object(:leaf; scale=:Leaf, status=original); + applications=( + ModelSpec( + InitialStatusPreparationProbe( + (supplied=Required(Real), offset=Default(-1.0)), + (result=-2.0,), + ); name=:first, on=One(scale=:Leaf), + ), + ModelSpec( + InitialStatusPreparationProbe((offset=Default(-3.0),), NamedTuple()); + name=:second, on=One(scale=:Leaf), + ), + ), + ) + compiled = Advanced.refresh_bindings!(model) + @test initial_preparation_status(model, :leaf) === original + @test propertynames(original) == (:supplied, :alias, :result, :offset) + @test original.offset == 3.0 + @test PlantSimEngine.refvalue(original, :supplied) === supplied_reference + @test PlantSimEngine.refvalue(original, :alias) === supplied_reference + @test PlantSimEngine.refvalue(original, :result) === result_reference + for application_id in (:first, :second) + view = compiled.status_views_by_target[(application_id, ObjectId(:leaf))] + @test view.status === original + @test view.canonical_status === original + @test isempty(view.temporal_inputs) + @test isempty(view.private_outputs) + end + + # Both directions must still reach the caller's storage after preparation. + storage[1] = 9.25 + @test original.supplied == original.alias == 9.25 + original.result = 4.5 + @test storage[2] == 4.5 + simulation = run!(model; steps=1, outputs=:none) + @test initial_preparation_status(model, :leaf) === original + @test final_state(simulation).supplied == 9.25 + @test storage == [9.25, 4.5] +end + +@testset "empty model ports still create and validate object statuses" begin + probe = InitialStatusPreparationProbe(NamedTuple(), NamedTuple()) + supplied = Status() + model = CompositeModel( + Object(:supplied; scale=:Leaf, status=supplied), + Object(:missing_a; scale=:Leaf), + Object(:missing_b; scale=:Leaf); + applications=(ModelSpec(probe; name=:empty, on=Many(scale=:Leaf)),), + ) + compiled = Advanced.refresh_bindings!(model) + @test initial_preparation_status(model, :supplied) === supplied + for object in model_objects(model) + @test object.status isa Status + @test isempty(propertynames(object.status)) + @test object_id(model, object.status) == object.id + view = compiled.status_views_by_target[(:empty, object.id)] + @test view.status === object.status + @test view.canonical_status === object.status + end + @test initial_preparation_status(model, :missing_a) !== + initial_preparation_status(model, :missing_b) + + invalid_status = (sentinel=7.0,) + invalid = CompositeModel( + Object(:invalid; scale=:Leaf, status=invalid_status); + applications=(ModelSpec(probe; name=:empty, on=One(scale=:Leaf)),), + ) + @test_throws "Model object `invalid` uses model applications but its status has type" Advanced.refresh_bindings!(invalid) + @test initial_preparation_status(invalid, :invalid) === invalid_status +end + @testset "initial preparation preserves existing references and first defaults" begin original = Status(signal=7.0, untouched=[8.0]) signal_reference = PlantSimEngine.refvalue(original, :signal) @@ -134,6 +216,50 @@ end @test final_state(simulation, :leaf_b).observed == 14.0 end +@testset "initial bound defaults retain conversion evidence and source identity" begin + transformed = Symbol[] + transform = (variable, value) -> begin + push!(transformed, variable) + variable == :bound ? 2 * value : value + end + model = CompositeModel( + Object(:source; scale=:Source), + Object(:leaf; scale=:Leaf); + applications=( + ModelSpec( + InitialStatusPreparationProbe(NamedTuple(), (signal=5.0,)); + name=:source, on=One(scale=:Source), + ), + ModelSpec( + InitialStatusPreparationProbe((bound=Default(-99.0),), NamedTuple()); + name=:reader, on=One(scale=:Leaf), + inputs=(bound=One(scale=:Source, within=SceneScope(), + application=:source, var=:signal),), + ), + ), + type_promotion=Dict(Float64 => Float32), + status_transform=transform, + ) + compiled = Advanced.refresh_bindings!(model) + source = initial_preparation_status(model, :source) + reader = initial_preparation_status(model, :leaf) + @test reader.bound === source.signal === 5.0f0 + @test PlantSimEngine.refvalue(reader, :bound) === + PlantSimEngine.refvalue(source, :signal) + @test count(==(:bound), transformed) == 1 + row = only(row for row in explain_initialization(model) + if row.application_id == :reader && row.role == :input && row.variable == :bound) + @test row.disposition == :producer_bound + @test row.original_type === Float64 + @test row.effective_type === Float32 + @test row.type_mapping_applied + @test row.status_transform_applied + @test row.status_transform_changed + view = compiled.status_views_by_target[(:reader, ObjectId(:leaf))] + @test view.status === reader + @test view.canonical_status === reader +end + @testset "initial defaults preserve numeric conversion diagnostics" begin transform = (variable, value) -> variable == :offset ? 2 * value : value probe = InitialStatusPreparationProbe( @@ -184,13 +310,20 @@ end ) original = only(model_objects(model)).status kept_reference = PlantSimEngine.refvalue(original, :kept) - Advanced.refresh_bindings!(model) + compiled = Advanced.refresh_bindings!(model) status = only(model_objects(model)).status @test propertynames(status) == (:kept,) @test status.kept == 7.0 @test PlantSimEngine.refvalue(status, :kept) === kept_reference @test !hasproperty(status, :scratch) + view = compiled.status_views_by_target[(:private_output, ObjectId(:leaf))] + @test view.status !== status + @test view.canonical_status === status + @test PlantSimEngine.refvalue(view.status, :kept) !== kept_reference + @test view.status.kept == 1.0 + @test view.status.scratch == [2.0] + simulation = run!(model; steps=1, outputs=:all) @test !hasproperty(only(model_objects(model)).status, :scratch) @test last(outputs(simulation)[(:private_output, ObjectId(:leaf), :scratch)])[2] == [2.0]