Port of PSI cost expression refactor#108
Open
acostarelli wants to merge 2 commits intomainfrom
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Ports PSI’s cost-expression refactor into POM/PSI by bundling cost expression container creation, expanding thermal cost decomposition, and adding renewable curtailment-cost tracking.
Changes:
- Replaced scattered
add_expressions!calls with centralizedadd_cost_expressions!across device constructors. - Added standalone renewable
CurtailmentCostExpressionand objective contribution viaadd_curtailment_cost!. - Updated tests to reflect expanded expression exports and validate decomposition/nonnegativity.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/test_model_decision.jl | Adjusts expectations for increased exported expressions and outputs. |
| test/test_device_thermal_generation_constructors.jl | Adds decomposition checks and startup-cost attribution checks. |
| test/test_device_renewable_generation_constructors.jl | Adds nonnegativity test for CurtailmentCostExpression. |
| src/static_injector_models/thermalgeneration_constructor.jl | Switches to add_cost_expressions! for thermal devices. |
| src/static_injector_models/source_constructor.jl | Switches to add_cost_expressions! for sources. |
| src/static_injector_models/renewablegeneration_constructor.jl | Switches to add_cost_expressions! for renewables. |
| src/static_injector_models/renewable_generation.jl | Adds curtailment cost into renewable objective. |
| src/static_injector_models/load_constructor.jl | Switches to add_cost_expressions! for loads. |
| src/common_models/objective_function.jl | Implements renewable curtailment cost objective terms. |
| src/common_models/market_bid_plumbing.jl | Routes VOM proportional cost to VOMCostExpression. |
| src/common_models/add_to_expression.jl | Refactors fuel-consumption expression building and compact formulation handling. |
| src/common_models/add_expressions.jl | Adds add_cost_expressions! bundles and fuel/polynomial predicates. |
| src/PowerOperationsModels.jl | Imports/exports additional cost expression types and includes new objective helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # treat as no-op rather than erroring so renewables with PWL/other costs still build. | ||
| _add_curtailment_cost!( | ||
| ::OptimizationContainer, ::Type{<:VariableType}, ::PSY.RenewableGen, | ||
| ::PSY.OperationalCost, ::Type{<:AbstractDeviceFormulation}) = nothing |
Comment on lines
+2733
to
+2751
| for t in time_steps | ||
| sos_status = _get_sos_value(container, W, d) | ||
| if sos_status == SOSStatusVariable.NO_VARIABLE | ||
| JuMP.add_to_expression!(expression[name, t], P_min * prop_term_per_unit * dt) | ||
| elseif sos_status == SOSStatusVariable.PARAMETER | ||
| param = get_default_on_parameter(d) | ||
| bin = get_parameter(container, param, V).parameter_array[name, t] | ||
| JuMP.add_to_expression!( | ||
| expression[name, t], P_min * prop_term_per_unit * dt, bin) | ||
| elseif sos_status == SOSStatusVariable.VARIABLE | ||
| var = get_default_on_variable(d) | ||
| bin = get_variable(container, var, V)[name, t] | ||
| JuMP.add_to_expression!( | ||
| expression[name, t], P_min * prop_term_per_unit * dt, bin) | ||
| else | ||
| @assert false | ||
| end | ||
| JuMP.add_to_expression!( | ||
| expression[name, t], prop_term_per_unit * dt, variable[name, t]) |
Comment on lines
+139
to
+145
| n = length(devices) | ||
| all_names = Vector{String}(undef, n) | ||
| fuel_names = sizehint!(String[], n) | ||
| has_quad_fuel = false | ||
| for (i, d) in enumerate(devices) | ||
| name = PSY.get_name(d) | ||
| all_names[i] = name |
| @test length(read_parameters(res; table_format = TableFormat.WIDE)) == 1 | ||
| @test length(read_duals(res; table_format = TableFormat.WIDE)) == 0 | ||
| @test length(read_expressions(res; table_format = TableFormat.WIDE)) == 2 | ||
| @test length(read_expressions(res; table_format = TableFormat.WIDE)) == 7 |
|
|
||
| @test length(readdir(IOM.export_realized_outputs(outputs1))) === 7 | ||
| exported = readdir(IOM.export_realized_outputs(outputs1)) | ||
| @test length(exported) >= 12 |
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.
Sienna-Platform/PowerSimulations.jl#1564
Requires IOM: Sienna-Platform/InfrastructureOptimizationModels.jl#94