From 160f4ccdddc3eb8b4ace05770433ef6dbec1d983 Mon Sep 17 00:00:00 2001 From: Luke Kiernan Date: Thu, 16 Jul 2026 17:07:36 -0600 Subject: [PATCH 1/4] Replace SystemUnitsSettings with a plain Float64 units anchor The unit_system field on SystemUnitsSettings was a leftover from the stateful-units era: it made the struct mutable and shared by reference across every attached component so a system-wide unit change would propagate instantly, but that whole mechanism is being retired (see matching PowerSystems.jl change). The only thing components genuinely need is the system's base power, a plain number that never changes after construction. InfrastructureSystemsInternal.units_info is now Union{Nothing, Float64}. get_units_info/set_units_info! are renamed get_base_value/set_base_value! and gain a generic default (get_base_value(x) = get_base_value(get_internal(x))) so any type implementing get_internal picks it up for free, and a type that stores its own anchor directly (no InfrastructureSystemsInternal) can add one small concrete method instead of a parallel mechanism. Co-Authored-By: Claude Sonnet 5 --- src/internal.jl | 30 ++++++++++++++---------------- src/utils/print.jl | 8 -------- test/test_units.jl | 12 +++++++----- 3 files changed, 21 insertions(+), 29 deletions(-) diff --git a/src/internal.jl b/src/internal.jl index 2ded96f67..0c7240d21 100644 --- a/src/internal.jl +++ b/src/internal.jl @@ -1,8 +1,6 @@ import UUIDs -abstract type UnitsData end - @scoped_enum(UnitSystem, SYSTEM_BASE = 0, DEVICE_BASE = 1, NATURAL_UNITS = 2,) @doc """ @@ -14,14 +12,6 @@ Unit system for component data values. - `NATURAL_UNITS`: Values in natural units (e.g., MW, MVAR) """ UnitSystem -@kwdef mutable struct SystemUnitsSettings <: UnitsData - const base_value::Float64 - unit_system::UnitSystem -end - -serialize(val::SystemUnitsSettings) = serialize_struct(val) -deserialize(T::Type{<:SystemUnitsSettings}, val::Dict) = deserialize_struct(T, val) - @kwdef struct SharedSystemReferences <: InfrastructureSystemsType supplemental_attribute_manager::Union{Nothing, AbstractSupplementalAttributeManager} = nothing @@ -34,7 +24,7 @@ Internal storage common to InfrastructureSystems types. mutable struct InfrastructureSystemsInternal <: InfrastructureSystemsType uuid::Base.UUID shared_system_references::Union{Nothing, SharedSystemReferences} - units_info::Union{Nothing, SystemUnitsSettings} + units_info::Union{Nothing, Float64} ext::Union{Nothing, Dict{String, Any}} end @@ -85,8 +75,16 @@ function set_shared_system_references!( return end -get_units_info(internal::InfrastructureSystemsInternal) = internal.units_info -set_units_info!(internal::InfrastructureSystemsInternal, val) = internal.units_info = val +get_base_value(internal::InfrastructureSystemsInternal) = internal.units_info +set_base_value!(internal::InfrastructureSystemsInternal, val) = internal.units_info = val + +""" +Generic accessor for the base-value units anchor: works for anything implementing +`get_internal`. Types that store their own anchor directly (rather than through an +`InfrastructureSystemsInternal`) should add a concrete method instead. +""" +get_base_value(x) = get_base_value(get_internal(x)) +set_base_value!(x, val) = set_base_value!(get_internal(x), val) """ Gets the UUID for any InfrastructureSystemsType. @@ -108,9 +106,9 @@ function serialize(internal::InfrastructureSystemsInternal) for field in fieldnames(InfrastructureSystemsInternal) val = getproperty(internal, field) - # reset the units data since this is a struct related to the system the components is - # added which is resolved later in the de-serialization. - if val isa UnitsData + # units_info is resolved against the system the component is added to, later, at + # deserialization time - never serialize the live value. + if field == :units_info val = nothing elseif field == :shared_system_references continue diff --git a/src/utils/print.jl b/src/utils/print.jl index 65698359f..84401004e 100644 --- a/src/utils/print.jl +++ b/src/utils/print.jl @@ -91,14 +91,6 @@ function Base.show(io::IO, ::MIME"text/html", data::SystemData) show_supplemental_attributes_data(io, data; backend = :html, stand_alone = false) end -function Base.show(io::IO, ::MIME"text/plain", system_units::SystemUnitsSettings) - print(io, summary(system_units), ":") - for name in fieldnames(typeof(system_units)) - val = getproperty(system_units, name) - print(io, "\n ", name, ": ", val) - end -end - function Base.show(io::IO, ::MIME"text/plain", ist::TimeSeriesOwners) print(io, summary(ist), ":") for name in fieldnames(typeof(ist)) diff --git a/test/test_units.jl b/test/test_units.jl index 28f3a0396..c231be5ef 100644 --- a/test/test_units.jl +++ b/test/test_units.jl @@ -29,9 +29,11 @@ end end end -@testset "SystemUnitsSettings base_value is immutable" begin - settings = IS.SystemUnitsSettings(100.0, IS.UnitSystem.SYSTEM_BASE) - @test_throws ErrorException settings.base_value = 50.0 - settings.unit_system = IS.UnitSystem.NATURAL_UNITS - @test settings.unit_system == IS.UnitSystem.NATURAL_UNITS +@testset "InfrastructureSystemsInternal units-anchor accessors" begin + internal = IS.InfrastructureSystemsInternal() + @test IS.get_base_value(internal) === nothing + IS.set_base_value!(internal, 100.0) + @test IS.get_base_value(internal) === 100.0 + IS.set_base_value!(internal, nothing) + @test IS.get_base_value(internal) === nothing end From 57baf761e92c9d67247a6675e37ca8ebfdb9c994 Mon Sep 17 00:00:00 2001 From: Luke Kiernan Date: Fri, 17 Jul 2026 10:03:42 -0600 Subject: [PATCH 2/4] fix docs; rename `units_info` field to `base_value` --- .../src/docs_best_practices/how-to/troubleshoot.md | 2 +- .../docs_best_practices/how-to/write_a_tutorial.md | 2 +- src/internal.jl | 14 +++++++------- src/time_series_metadata_store.jl | 8 ++++---- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/docs/src/docs_best_practices/how-to/troubleshoot.md b/docs/src/docs_best_practices/how-to/troubleshoot.md index 995c1e63b..c520c9b5e 100644 --- a/docs/src/docs_best_practices/how-to/troubleshoot.md +++ b/docs/src/docs_best_practices/how-to/troubleshoot.md @@ -82,7 +82,7 @@ Pkg.add(["PowerSystems"]); 2. Run the formatter once to format the rest of the file 3. Add the text back in 4. Add the `ignore` keyword argument with the file name to - [`JuliaFormatter.format`](@extref `JuliaFormatter.format-Tuple{Any}`) in `scripts/formatter/formatter_code.jl` + [`JuliaFormatter.format`](@extref) in `scripts/formatter/formatter_code.jl` to skip the file in the future: ```julia diff --git a/docs/src/docs_best_practices/how-to/write_a_tutorial.md b/docs/src/docs_best_practices/how-to/write_a_tutorial.md index 545fdd6c4..5445a0015 100644 --- a/docs/src/docs_best_practices/how-to/write_a_tutorial.md +++ b/docs/src/docs_best_practices/how-to/write_a_tutorial.md @@ -56,7 +56,7 @@ code blocks. !!! tip "Do" Display all code, starting from `using SomeSiennaPackage`. Example: See - [Working with Time Series](@extref tutorial_time_series). + [Working with Time Series](@extref PowerSystems :doc:`tutorials/generated_working_with_time_series`). !!! warning "Don't" diff --git a/src/internal.jl b/src/internal.jl index 0c7240d21..1b01e0119 100644 --- a/src/internal.jl +++ b/src/internal.jl @@ -24,7 +24,7 @@ Internal storage common to InfrastructureSystems types. mutable struct InfrastructureSystemsInternal <: InfrastructureSystemsType uuid::Base.UUID shared_system_references::Union{Nothing, SharedSystemReferences} - units_info::Union{Nothing, Float64} + base_value::Union{Nothing, Float64} ext::Union{Nothing, Dict{String, Any}} end @@ -34,10 +34,10 @@ Creates InfrastructureSystemsInternal with a new UUID. InfrastructureSystemsInternal(; uuid = make_uuid(), shared_system_references = nothing, - units_info = nothing, + base_value = nothing, ext = nothing, ) = - InfrastructureSystemsInternal(uuid, shared_system_references, units_info, ext) + InfrastructureSystemsInternal(uuid, shared_system_references, base_value, ext) """ Creates InfrastructureSystemsInternal with an existing UUID. @@ -75,8 +75,8 @@ function set_shared_system_references!( return end -get_base_value(internal::InfrastructureSystemsInternal) = internal.units_info -set_base_value!(internal::InfrastructureSystemsInternal, val) = internal.units_info = val +get_base_value(internal::InfrastructureSystemsInternal) = internal.base_value +set_base_value!(internal::InfrastructureSystemsInternal, val) = internal.base_value = val """ Generic accessor for the base-value units anchor: works for anything implementing @@ -106,9 +106,9 @@ function serialize(internal::InfrastructureSystemsInternal) for field in fieldnames(InfrastructureSystemsInternal) val = getproperty(internal, field) - # units_info is resolved against the system the component is added to, later, at + # base_value is resolved against the system the component is added to, later, at # deserialization time - never serialize the live value. - if field == :units_info + if field == :base_value val = nothing elseif field == :shared_system_references continue diff --git a/src/time_series_metadata_store.jl b/src/time_series_metadata_store.jl index 7ad76da49..2af775f29 100644 --- a/src/time_series_metadata_store.jl +++ b/src/time_series_metadata_store.jl @@ -157,8 +157,8 @@ function _load_metadata_into_memory_legacy!(store::TimeSeriesMetadataStore) if !isnothing(internal.ext) && !isempty(internal.ext) @warn "ext is no longer supported on a time series metadata instance and will be dropped: $(internal.ext)" end - if !isnothing(internal.units_info) - @warn "units_info is no longer supported on a time series metadata instance and will be dropped: $(internal.units_info)" + if !isnothing(internal.base_value) + @warn "base_value is no longer supported on a time series metadata instance and will be dropped: $(internal.base_value)" end uuid = get_uuid(metadata) if haskey(store.metadata_uuids, uuid) @@ -492,9 +492,9 @@ function add_metadata!( if !isnothing(internal.ext) && !isempty(internal.ext) error("ext cannot be set on a time series metadata instance: $(internal.ext)") end - if !isnothing(internal.units_info) + if !isnothing(internal.base_value) error( - "units_info cannot be set on a time series metadata instance: $(internal.units_info)", + "base_value cannot be set on a time series metadata instance: $(internal.base_value)", ) end vals = _create_row( From 112422e220d0f68cdb5a86276e5490057a67a46d Mon Sep 17 00:00:00 2001 From: Luke Kiernan Date: Fri, 17 Jul 2026 13:48:57 -0600 Subject: [PATCH 3/4] Address Copilot review on PR #596 - Constrain get_base_value/set_base_value! generic fallback to InfrastructureSystemsType, matching the get_uuid convention, so it doesn't dispatch on unrelated types that happen to define get_internal. - Add test coverage for the generic get_internal-forwarding path via a minimal mock type, not just the InfrastructureSystemsInternal-direct methods. - Fix an @extref reference in write_a_tutorial.md that got mangled into invalid :doc: role syntax during the units_info->base_value rename commit; restore the working label-based reference. Co-Authored-By: Claude Sonnet 5 --- .../docs_best_practices/how-to/write_a_tutorial.md | 2 +- src/internal.jl | 4 ++-- test/test_units.jl | 14 ++++++++++++++ 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/docs/src/docs_best_practices/how-to/write_a_tutorial.md b/docs/src/docs_best_practices/how-to/write_a_tutorial.md index 5445a0015..545fdd6c4 100644 --- a/docs/src/docs_best_practices/how-to/write_a_tutorial.md +++ b/docs/src/docs_best_practices/how-to/write_a_tutorial.md @@ -56,7 +56,7 @@ code blocks. !!! tip "Do" Display all code, starting from `using SomeSiennaPackage`. Example: See - [Working with Time Series](@extref PowerSystems :doc:`tutorials/generated_working_with_time_series`). + [Working with Time Series](@extref tutorial_time_series). !!! warning "Don't" diff --git a/src/internal.jl b/src/internal.jl index 1b01e0119..da238dfbb 100644 --- a/src/internal.jl +++ b/src/internal.jl @@ -83,8 +83,8 @@ Generic accessor for the base-value units anchor: works for anything implementin `get_internal`. Types that store their own anchor directly (rather than through an `InfrastructureSystemsInternal`) should add a concrete method instead. """ -get_base_value(x) = get_base_value(get_internal(x)) -set_base_value!(x, val) = set_base_value!(get_internal(x), val) +get_base_value(x::InfrastructureSystemsType) = get_base_value(get_internal(x)) +set_base_value!(x::InfrastructureSystemsType, val) = set_base_value!(get_internal(x), val) """ Gets the UUID for any InfrastructureSystemsType. diff --git a/test/test_units.jl b/test/test_units.jl index c231be5ef..34f27668e 100644 --- a/test/test_units.jl +++ b/test/test_units.jl @@ -37,3 +37,17 @@ end IS.set_base_value!(internal, nothing) @test IS.get_base_value(internal) === nothing end + +struct MockUnitsAnchorHolder <: IS.InfrastructureSystemsType + internal::IS.InfrastructureSystemsInternal +end +IS.get_internal(x::MockUnitsAnchorHolder) = x.internal + +@testset "get_base_value/set_base_value! generic forwarding via get_internal" begin + mock = MockUnitsAnchorHolder(IS.InfrastructureSystemsInternal()) + @test IS.get_base_value(mock) === nothing + IS.set_base_value!(mock, 100.0) + @test IS.get_base_value(mock) === 100.0 + IS.set_base_value!(mock, nothing) + @test IS.get_base_value(mock) === nothing +end From 8c46639de207b3b5b5049491913183499e09cce5 Mon Sep 17 00:00:00 2001 From: Luke Kiernan Date: Mon, 20 Jul 2026 10:10:07 -0600 Subject: [PATCH 4/4] Copilot suggestion broke docs; put back --- docs/src/docs_best_practices/how-to/write_a_tutorial.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/docs_best_practices/how-to/write_a_tutorial.md b/docs/src/docs_best_practices/how-to/write_a_tutorial.md index 545fdd6c4..5445a0015 100644 --- a/docs/src/docs_best_practices/how-to/write_a_tutorial.md +++ b/docs/src/docs_best_practices/how-to/write_a_tutorial.md @@ -56,7 +56,7 @@ code blocks. !!! tip "Do" Display all code, starting from `using SomeSiennaPackage`. Example: See - [Working with Time Series](@extref tutorial_time_series). + [Working with Time Series](@extref PowerSystems :doc:`tutorials/generated_working_with_time_series`). !!! warning "Don't"