Price calculation using dispatch without candidates#1389
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1389 +/- ##
==========================================
- Coverage 89.60% 89.57% -0.04%
==========================================
Files 58 58
Lines 8338 8351 +13
Branches 8338 8351 +13
==========================================
+ Hits 7471 7480 +9
- Misses 550 554 +4
Partials 317 317 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Aligns commodity price calculation with the flows reported in commodity_flows.csv by basing cost-derived prices on the “final without candidates” dispatch solution, while still using the “final with candidates” solution for shadow prices (per #1364).
Changes:
- Split price calculation inputs into
solution_without_candidatesvssolution_with_candidatesand route each pricing strategy to the appropriate source solution. - Adjust dispatch-year execution to always compute flows from the “without candidates” run and shadow prices from the “with candidates” run (when candidates exist).
- Update expected CSV outputs for prices/debug appraisal across example models/tests.
Reviewed changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/simulation.rs | Uses separate dispatch runs for flows vs shadow-prices and wires both solutions into pricing. |
| src/simulation/prices.rs | Updates pricing pipeline to accept both dispatch solutions and use the correct one per strategy. |
| tests/data/two_regions/commodity_prices.csv | Updates expected prices to reflect new pricing inputs. |
| tests/data/simple/debug_appraisal_results.csv | Updates expected debug appraisal outputs from dispatch/pricing changes. |
| tests/data/simple/commodity_prices.csv | Updates expected commodity prices for the simple model. |
| tests/data/simple_shadow/commodity_prices.csv | Updates expected commodity prices for the shadow-pricing variant. |
| tests/data/simple_npv/commodity_prices.csv | Updates expected commodity prices for the NPV variant. |
| tests/data/simple_marginal/commodity_prices.csv | Updates expected commodity prices for marginal pricing variant. |
| tests/data/simple_marginal_average/commodity_prices.csv | Updates expected commodity prices for marginal-average variant. |
| tests/data/simple_ironing_out/commodity_prices.csv | Updates expected prices for ironing-out convergence scenario. |
| tests/data/simple_full/commodity_prices.csv | Updates expected prices for full-cost variant. |
| tests/data/simple_divisible/commodity_prices.csv | Updates expected prices for divisible-assets variant. |
| tests/data/muse1_default/commodity_prices.csv | Updates expected prices for the muse1_default example where results change most. |
| tests/data/muse1_default/assets.csv | Removes an asset row consistent with updated scenario outputs. |
| tests/data/muse1_default/asset_capacities.csv | Updates capacities to match revised dispatch/pricing behaviour and asset set. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let prices = match ( | ||
| solution_existing.as_ref(), | ||
| solution_with_candidates.as_ref(), | ||
| ) { | ||
| (None, None) => Prices::default(), | ||
| (Some(existing), None) => calculate_prices(model, existing, existing, year)?, | ||
| (Some(existing), Some(with_candidates)) => { | ||
| calculate_prices(model, existing, with_candidates, year)? | ||
| } | ||
| (None, Some(_)) => unreachable!("candidates without existing assets"), | ||
| }; |
There was a problem hiding this comment.
True... hadn't thought about having no existing assets in the base year. I guess we should keep the behaviour as it was, but to be honest I'm not sure if the price calculation code is really set up to handle this scenario. Maybe we need a test for this (i.e a model with no existing assets in the base year (and no demand))? Seem to recall at some point that this was something we explicitly wanted to allow
Description
Implements the changes discussed in #1364 to line up prices with the flows reported in
commodity_flows.csv.It's a bit fiddly because we now have to pass around both
solution_without_candidatesandsolution_with_candidateswhen calculating prices (we need the latter for shadow prices and to get activity keys for candidates). I wasn't quite sure about scarcity prices, so I kept it as it was before (using the solution with candidates).Mostly just small changes to prices in the example models. The only model where this substantially changes results is
muse1_default- not really sure why, but I guess no reason that tiny price changes couldn't impact investment decisions. Some of the price changes actually look a bit more substantial in this model compared to others.Fixes #1364
Type of change
Key checklist
$ cargo test$ cargo docpresent in the previous release
Further checks