add stubs for 2 edge cases where need PSY bus types#88
add stubs for 2 edge cases where need PSY bus types#88luke-kiernan wants to merge 4 commits intomainfrom
Conversation
|
Performance Results This branch |
There was a problem hiding this comment.
Pull request overview
This PR adds lightweight interface “stubs” intended to let downstream packages (e.g., POM) supply PowerSystems (PSY) bus component types without adding a PSY dependency here, and uses those stubs to handle two bus-typed edge cases in constraints/duals.
Changes:
- Route network-dual component collection through
component_for_network_dual(...)instead of hard-coding a PSY type. - Special-case
DCVoltageincremental interpolation to fetch the original variable from an HVDC bus component type viacomponent_for_hvdc_interpolation(...). - Introduce new interface hooks in
interfaces.jlfor downstream implementation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/quadratic_approximations/incremental.jl |
Uses a new helper to select the component type for DCVoltage variable lookup during incremental PWL interpolation. |
src/common_models/interfaces.jl |
Adds new interface hooks intended for downstream packages to provide PSY bus component types. |
src/common_models/add_constraint_dual.jl |
Switches network-dual component selection to use the new interface hook (intended to resolve ACBus typing without PSY dependency). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
IOM now declares stubs for the system-query interface it needs and calls them unqualified; downstream packages (POM) provide methods that forward to their system type's public API. No more sys.data access in IOM src. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Required interface methods - extend InfrastructureOptimizationModels functions for duck-typing | ||
| IOM.get_base_power(sys::MockSystem) = sys.data.base_power | ||
| IOM.stores_time_series_in_memory(sys::MockSystem) = sys.data.stores_in_memory | ||
| IOM.stores_time_series_in_memory(data::MockSystemData) = data.stores_in_memory | ||
| InfrastructureOptimizationModels.get_base_power(sys::MockSystem) = sys.base_power | ||
| InfrastructureOptimizationModels.stores_time_series_in_memory(sys::MockSystem) = | ||
| sys.stores_in_memory | ||
|
|
||
| function IOM.get_available_components(::NetworkModel, ::Type{T}, sys::MockSystem) where {T} | ||
| return get_components(T, sys) | ||
| end |
There was a problem hiding this comment.
MockSystem is updated to support get_base_power/stores_time_series_in_memory, but it doesn’t implement any of the newly introduced system-query extension points (e.g., get_system_uuid, get_time_series_resolutions, get_time_series_counts, get_forecast_interval/horizon, etc.). If future lightweight tests start building DecisionModel/EmulationModel (or any code path that calls these stubs), they will fail with MethodError. Consider adding simple MockSystem implementations for the new stubs (even if they just return empty summaries / a fixed UUID) to keep mocks aligned with the stated interface.
There was a problem hiding this comment.
okay this seems like something potentially worth addressing...but imo it's a part of a deeper problem, which is that emulation models (and to a lesser degree, decision models) aren't well-integrated or well-tested yet. I'd prefer to do that in a separate PR
| function get_time_series_resolutions end | ||
|
|
||
| "Extension point: counts summary of time series on the system." | ||
| function get_time_series_counts end | ||
|
|
||
| "Extension point: counts by component type of time series on the system." | ||
| function get_time_series_counts_by_type end | ||
|
|
||
| "Extension point: forecast interval configured on the system." | ||
| function get_forecast_interval end | ||
|
|
||
| "Extension point: forecast horizon configured on the system." | ||
| function get_forecast_horizon end | ||
|
|
||
| "Extension point: summary table of forecasts on the system." | ||
| function get_forecast_summary_table end | ||
|
|
||
| "Extension point: transform single time series into deterministic forecasts on the system." | ||
| function transform_single_time_series! end | ||
|
|
There was a problem hiding this comment.
I think several of these will cause name clashes with IS no ? Or are methods that should live in IS
Two small POM test fixes, needed to make tests pass on POM's MBC/IEC PR. Might be a cleaner way--change POM to conform to IOM instead of vice versa--but this'll do for now.