Move operation models to PowerOperationsModels#100
Closed
luke-kiernan wants to merge 2 commits into
Closed
Conversation
IOM keeps only abstract types and the operation-model interface; concrete
DecisionModel/EmulationModel, their stores, OptimizationProblemOutputs,
and related operation utilities now live in POM.
- Parameterize OperationModel{T <: OperationProblem}; collapse _get_model_type
- Move src/operation/{decision,emulation}_model.jl + stores, problem_outputs.jl,
time_series_interface.jl, store_common.jl, initial_conditions_update_in_memory_store.jl,
optimization_debugging.jl, model_numerical_analysis_utils.jl
- Move src/core/optimization_problem_outputs{,_export}.jl
- Slim operation_model_interface.jl: keep abstract-only accessors, hand POM the
store-reading and numerical-bounds helpers
- Drop dead test files and POM-glue mocks from test/
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
Contributor
|
Performance Results This branch |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors InfrastructureOptimizationModels (IOM) to retain only the abstract operation-model interface and shared key/container infrastructure, while moving concrete operation-model implementations (DecisionModel/EmulationModel), model stores, OptimizationProblemOutputs, and related utilities into PowerOperationsModels (POM).
Changes:
- Removed concrete operation model types/stores, output reading/exporting, numerical-bound/debug utilities, and time-series helpers from IOM (to be owned by POM).
- Introduced
OperationProblemas a common supertype and parameterizedOperationModel{T <: OperationProblem}; addedget_problem_typehelper. - Updated test harness and pretty-printing to reflect the new ownership boundaries.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_utils/run_simulation.jl | Removed simulation helper (operation models moved out). |
| test/test_utils/common_operation_model.jl | Removed IC serialization test helper (moved out with models/stores). |
| test/test_optimization_outputs.jl | Removed OptimizationProblemOutputs tests (now expected in POM). |
| test/test_model_store.jl | Removed placeholder model-store tests. |
| test/test_model_emulation.jl | Removed emulation model tests (now expected in POM). |
| test/InfrastructureOptimizationModelsTests.jl | Stops including moved tests; updates notes about test location/ownership. |
| src/utils/print_pt_v3.jl | Switches model type display to get_problem_type; removes show methods for moved concrete types. |
| src/operation/time_series_interface.jl | Removed time-series cache accessors for Decision/Emulation models. |
| src/operation/store_common.jl | Removed output-writing/export plumbing tied to concrete stores. |
| src/operation/problem_outputs.jl | Removed constructors for OptimizationProblemOutputs from Decision/Emulation models. |
| src/operation/optimization_debugging.jl | Removed MOI index/debug helpers tied to concrete models. |
| src/operation/operation_model_interface.jl | Trims to abstract-only getters/setters; removes store-reading/output helper methods. |
| src/operation/model_numerical_analysis_utils.jl | Removed numerical-bound analysis helpers (moved to POM). |
| src/operation/initial_conditions_update_in_memory_store.jl | Removed IC update-from-store stub (moved to POM). |
| src/operation/emulation_model.jl | Removed EmulationModel implementation (moved to POM). |
| src/operation/emulation_model_store.jl | Removed EmulationModelStore implementation (moved to POM). |
| src/operation/decision_model.jl | Removed DecisionModel implementation (moved to POM). |
| src/operation/decision_model_store.jl | Removed DecisionModelStore implementation (moved to POM). |
| src/core/optimization_problem_outputs.jl | Removed OptimizationProblemOutputs type + readers/export/serialization (moved to POM). |
| src/core/optimization_problem_outputs_export.jl | Removed export-configuration type (moved to POM). |
| src/core/operation_model_abstract_types.jl | Adds OperationProblem and parameterizes OperationModel; defines get_problem_type. |
| src/InfrastructureOptimizationModels.jl | Updates exports/includes to match new IOM/POM split; removes includes for moved code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Remove stale read_optimizer_stats export (function moved to POM). - Correct misleading comment in operation_model_interface.jl: instantiate_network_model!(::OperationModel) legitimately stays in IOM (interface-accessor entry point that dispatches to POM-owned concrete methods). - Fix _get_model_type → get_problem_type in print_pt_v3.jl emulator-row branch. - Delete unused Simulation/SimulationOutputs/SimulationProblemOutputs abstract type placeholders and their orphaned show methods (PSI types, never used here). - Bump PowerNetworkMatrices compat to ^0.20 and drop self dev-link from test/Project.toml. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Member
|
Why is this stuff moving to POM? |
jd-lara
reviewed
May 14, 2026
Member
There was a problem hiding this comment.
This file should stay here in IOM
jd-lara
reviewed
May 14, 2026
Member
|
@luke-kiernan this PR needs to be more carefully crafted |
Collaborator
Author
Hmm. My basic question: why is this operation stuff here in the first place? Organizationally, it'd fit better in POM. Putting it there minimizes the number of stubs, so we can write tests without extensive duck-typing. Once I started moving a couple things to POM, I found myself with structs and functions defined in IOM that are only used in POM. Which lead me to move more things to POM.
Just those two? If so, why just those two? Yeah the underlying organizational philosophy here is completely lost on me. |
Member
|
@luke-kiernan we will come back to this once #97 is merged |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DecisionModel/EmulationModel, their stores,OptimizationProblemOutputs, and related operation utilities to PowerOperationsModels.OperationModel{T <: OperationProblem}and addOperationProblemas a common supertype forDecisionProblem/EmulationProblem; collapse_get_model_typeto one method.operation_model_interface.jlto abstract-only accessors; POM owns the store-reading and numerical-bounds helpers.Pairs with POM PR (to follow). POM needs to land first locally; once merged, update POM's
[sources]entry to point at the merged IOM rev.Test plan
🤖 Generated with Claude Code