Skip to content

Absorb operation models from InfrastructureOptimizationModels#121

Open
luke-kiernan wants to merge 3 commits into
mainfrom
lk/absorb-operation-from-iom
Open

Absorb operation models from InfrastructureOptimizationModels#121
luke-kiernan wants to merge 3 commits into
mainfrom
lk/absorb-operation-from-iom

Conversation

@luke-kiernan
Copy link
Copy Markdown
Collaborator

Summary

  • Receive concrete DecisionModel/EmulationModel, their stores, OptimizationProblemOutputs, and operation-model glue methods (read_*, list_*_keys, _check_numerical_bounds, _pre_solve_model_checks) from IOM.
  • Deduplicate get_problem_type, validate_template, handle_initial_conditions! onto OperationModel{T}.
  • Pairs with IOM #100. `[sources]` points at IOM branch `lk/move-operation-to-pom`; update to the merged rev once IOM lands.

Test plan

  • Full `Pkg.test()`: 13109/13110 pass (1 pre-existing broken).
  • Re-resolve `[sources]` to the merged IOM rev before final merge.

🤖 Generated with Claude Code

Concrete DecisionModel/EmulationModel + stores, OptimizationProblemOutputs,
and the operation-model glue methods (read_*, list_*_keys, _check_numerical_bounds,
_pre_solve_model_checks) that compose POM-only types now live here. IOM keeps
just the abstract interface.

Deduplicate get_problem_type, validate_template, handle_initial_conditions!
onto OperationModel{T}. Pairs with IOM PR #100.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates operation model implementations and output/storage infrastructure from InfrastructureOptimizationModels (IOM) into PowerOperationsModels (POM), consolidating default DecisionModel / EmulationModel behavior and bringing OptimizationProblemOutputs + model-store logic into this repository.

Changes:

  • Adds OptimizationProblemOutputs (read/export/serialize helpers) and new Decision/Emulation model store implementations under src/operation/.
  • Refactors operation-model lifecycle glue (template validation dispatch, numerical-bounds checks, time series cache interface, initial-conditions update helpers) to live in POM.
  • Updates tests and test dependencies to validate the migrated outputs behavior and updated POM/IOM split.

Reviewed changes

Copilot reviewed 21 out of 22 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
src/PowerOperationsModels.jl Expands imports and include order to accommodate operation-model + outputs/store code moved from IOM.
src/utils/print.jl Adds show methods for problem outputs and numerical-bounds types.
src/operation/decision_model.jl Introduces concrete DecisionModel + default problem types and lifts shared logic onto OperationModel.
src/operation/emulation_model.jl Introduces concrete EmulationModel + default problem types and related update/build logic.
src/operation/decision_model_store.jl Adds store implementation for DecisionModel outputs.
src/operation/emulation_model_store.jl Adds store implementation for EmulationModel outputs (dataset-backed).
src/operation/store_common.jl Adds shared output-writing and export helpers, including path sanitization.
src/operation/problem_outputs.jl Adds constructors building OptimizationProblemOutputs from solved models.
src/operation/optimization_problem_outputs.jl Adds OptimizationProblemOutputs container + read/export/serialize APIs.
src/operation/optimization_problem_outputs_export.jl Adds export configuration struct for outputs export selection.
src/operation/optimization_debugging.jl Adds helpers to map MOI indices back to constraints/variables.
src/operation/model_numerical_analysis_utils.jl Adds numerical-bounds analysis utilities for constraints/variables.
src/operation/operation_model_glue.jl Adds abstract OperationModel glue methods for store reads/listing and pre-solve checks.
src/operation/template_validation.jl Centralizes default/custom validate_template dispatch for operation models.
src/operation/time_series_interface.jl Adds cached time series retrieval helpers for Decision/Emulation models.
src/operation/initial_conditions_update_in_memory_store.jl Adds initial-conditions update entrypoint for store-backed ICs.
Project.toml Adds new direct deps (CSV/DataFrames/HDF5/PrettyTables/etc.) and sources pin.
test/test_optimization_outputs.jl New tests for OptimizationProblemOutputs timestamp processing, reads, and natural-unit conversion.
test/test_model_decision.jl Updates decision model tests to use POM-exported APIs for outputs/keys/duals/parameters/bounds.
test/test_utils/mock_operation_models.jl Updates mock models to subtype POM default problem types and use moved methods.
test/test_utils/model_checks.jl Updates numerical-bounds test helpers to reference POM bounds types.
test/Project.toml Updates test sources pin to the migration branch of IOM.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/operation/emulation_model.jl Outdated
Comment thread src/operation/decision_model.jl Outdated
Comment thread src/operation/emulation_model_store.jl Outdated
Comment thread src/operation/emulation_model_store.jl Outdated

function list_all_keys(x::OperationModel)
return Iterators.flatten(
list_fields(get_store(x), T) for T in STORE_CONTAINER_TYPES
Copy link
Copy Markdown
Collaborator Author

@luke-kiernan luke-kiernan May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh! see this comment: iterating over STORE_CONTAINER_TYPES is better, but yes, then we need to make sure it's in scope (bc it's defined back in IOM)

Comment thread src/operation/optimization_problem_outputs.jl Outdated
Comment thread src/operation/optimization_problem_outputs.jl Outdated
Comment thread src/utils/print.jl Outdated
Comment thread src/utils/print.jl Outdated
Comment thread src/operation/optimization_problem_outputs.jl Outdated
All 10 CoPilot comments — real bugs in code carried over from IOM:
fix undefined `template` reference in EmulationModel constructor (mirror
DecisionModel's pattern), add missing `throw()` around an IS.ArgumentError,
remove dead empty!/isempty branches that referenced fields DatasetContainer
doesn't have, import the missing STORE_CONTAINER_TYPES, correct the
undefined `sys_uuid`/`res.source_uuid` in set_source_data!'s error
message, swap JSON for JSON3, fix the bare tf_html_simple and stray
ISOPT.get_resolution in print.jl, and patch docstring typos.

New regression tests caught two more: show(::OptimizationProblemOutputs)
referenced an undefined `T` and a missing `outputs.problem` field, and
export_optimizer_stats JSON path passed a DataFrame into to_dict which
has no method. Also fix the `bonuds` typo in 5 update_numerical_bounds
methods and FIXME the dead PSI-leftover should_export_* call paths.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator Author

@luke-kiernan luke-kiernan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at all definitions of write_output!, doeesn't look like name::Symbol is used anywhere. update_timestamp is only used for the emulation model ones.

"The System contains a combination of forecast data and transformed time series data. Currently this is not supported.",
)
end
if "Deterministic" ∈ existing_types
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-existing issue: is string matching really the best way to do this? It seems we ought to work with types directly instead...

template::AbstractProblemTemplate,
sys::IS.InfrastructureSystemsContainer,
jump_model::Union{Nothing, JuMP.Model} = nothing;
name = nothing,
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is begging for a macro or something similar

time_steps_count = get_time_steps(container)[end]
initial_time = get_initial_time(container)
model_interval = get_interval(params)
for type in STORE_CONTAINERS
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: we're iterating over symbols here. Iterating over types (STORE_CONTAINER_TYPES) would be more compilation-friendly, especially if we unroll the loop.

function write_output!(
store::DecisionModelStore,
name::Symbol,
key::OptimizationContainerKey,
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add type parameter: if key::OptimizationContainerKey{M, <:InfrastructureSystemsType} then we can write container = getfield(store, IOM.field_for_type(M)). Also applies to most of the below write_output! and read_output definitions too.

return
end

function get_current_time(model::EmulationModel)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

identical to the DecisionModel definition.

"""
Construct OptimizationProblemOutputs from a solved EmulationModel.
"""
function OptimizationProblemOutputs(model::EmulationModel)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically identical to the DecisionModel version. Consolidate.


# Aliases used for clarity in the method dispatches so it is possible to know if writing to
# DecisionModel data or EmulationModel data
# Note: DecisionModelIndexType and EmulationModelIndexType are defined in core/definitions.jl
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These constants are defined back in IOM, but only used in POM: move them over.

return
end

function write_model_dual_outputs!(
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again with the "nearly the same thing but with a different optimization key type." See if it can be easily consolidated via multiple dispatch or macro.

return TimeSeries.values(ts)
end

function get_time_series_values!(
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically the same as DecisionModel. Consolidate?

@@ -0,0 +1,337 @@
import PowerOperationsModels:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Find a less ad-hoc but still modular way to test this stuff? What's going on in this file seems complex enough to deserve better test cases than these superficial AI-written ones.

@luke-kiernan luke-kiernan requested a review from acostarelli May 13, 2026 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants