NPV: Avoid excessive candidate capacity during investment#1271
NPV: Avoid excessive candidate capacity during investment#1271
Conversation
We don't need to wait until adding the constraint before working out what the fallback value should be.
In practice, these appear to be the same, but we shouldn't rely on HiGHS doing this.
…ndidate assets Otherwise, when using NPV, the asset's capacity can end up exceeding requirements. Surprisingly, the capacities of candidate assets in the `two_regions` model were also affected in some places, although the model uses LCOX through, which suggests there is a bug. Fixes #1202.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1271 +/- ##
=======================================
Coverage 89.76% 89.77%
=======================================
Files 57 57
Lines 8210 8216 +6
Branches 8210 8216 +6
=======================================
+ Hits 7370 7376 +6
Misses 544 544
Partials 296 296 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR addresses excessive candidate capacity during tranche-based investment appraisal (notably for the NPV objective) by recalculating demand-limiting capacity (DLC) each appraisal round so candidate capacity bounds better reflect remaining unmet demand.
Changes:
- Recompute DLC during each appraisal round and bound candidate max capacity by
min(tranche_capacity, recalculated_dlc, remaining_candidate_capacity). - Simplify appraisal optimisation interfaces by unwrapping
max_capacityonce inappraise_investment(internal APIs now takeAssetCapacityinstead ofOption<AssetCapacity>). - Update regression fixtures for
two_regionsandsimple_npvto match the new investment/appraisal outcomes.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/simulation/investment.rs |
Recalculates DLC each selection round and tightens candidate capacity bounds during appraisal. |
src/simulation/investment/appraisal.rs |
Refactors max_capacity handling and propagates new optimisation signatures into NPV/LCOX appraisal. |
src/simulation/investment/appraisal/optimisation.rs |
Makes optimisation take a non-optional max_capacity bound. |
src/simulation/investment/appraisal/constraints.rs |
Updates capacity constraint helper to take a non-optional max_capacity. |
tests/data/two_regions/commodity_prices.csv |
Updates expected regression outputs for the two_regions model. |
tests/data/two_regions/commodity_flows.csv |
Updates expected regression outputs for the two_regions model. |
tests/data/two_regions/assets.csv |
Updates expected regression outputs for the two_regions model. |
tests/data/simple_npv/commodity_flows.csv |
Updates expected regression outputs for the simple_npv patched model. |
tests/data/simple_npv/assets.csv |
Updates expected regression outputs for the simple_npv patched model. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Ok(AppraisalOutput::new( | ||
| asset.clone(), | ||
| max_capacity, | ||
| results, | ||
| Some(NPVMetric::new(profitability_index)), | ||
| coefficients.clone(), | ||
| )) |
There was a problem hiding this comment.
In calculate_npv, AppraisalOutput::new is being given max_capacity rather than the optimiser's chosen results.capacity. This makes the output capacity inconsistent with the activity/unmet_demand/metric (which are all based on results), and can also cause AppraisalOutput::is_valid() to treat a solution with results.capacity == 0 as valid if max_capacity > 0 (potentially selecting an asset that doesn't actually serve demand). Use results.capacity for the output capacity, or alternatively change the optimisation formulation so results.capacity is forced to equal max_capacity if that is the intended installed capacity for NPV appraisal.
| for asset in &opt_assets { | ||
| // For candidates, determine the maximum capacity that can be invested in this round, | ||
| // according to the tranche size and remaining capacity limits. | ||
| // For candidates, determine the maximum capacity that can be invested in this round. | ||
| // This is whichever is the smallest of the tranche size (based on demand limiting | ||
| // capacity before investment), the remaining available capacity for the candidate and | ||
| // the demand limiting capacity recalculated based on demand unserved by the other | ||
| // selected assets. | ||
| let max_capacity = (!asset.is_commissioned()).then(|| { | ||
| let tranche_capacity = asset | ||
| .capacity() | ||
| .apply_limit_factor(model.parameters.capacity_limit_factor); | ||
| let dlc = AssetCapacity::from_capacity( | ||
| get_demand_limiting_capacity(&model.time_slice_info, asset, commodity, &demand), | ||
| asset.unit_size(), | ||
| ); | ||
| let remaining_capacity = remaining_candidate_capacity[asset]; | ||
| tranche_capacity.min(remaining_capacity) | ||
|
|
||
| tranche_capacity.min(dlc).min(remaining_capacity) | ||
| }); |
There was a problem hiding this comment.
max_capacity (including the new demand-limiting-capacity recomputation) is calculated before the group-id de-duplication check. If there are many identical candidates in the same group, this recomputes DLC/tranche/remaining for assets that will be skipped anyway. Consider moving the group check before computing max_capacity (or computing max_capacity only for the representative asset) to avoid unnecessary work in large models.
|
Actually, in retrospect, I'm not sure I'm right about the LCOX agent for Adam's model. Need to take a look at that again not in a rush before I get off the train! The two_regions model is definitely affected by this change though, which is odd. Could be a bug related to multi-region models or that could be a red herring. I also changed the NPV implementation to not use the dud capacity variable, like we talked about. |
Description
As discussed on #1202, we can sometimes get excessive capacity for candidate assets during the appraisal step, if there is not much demand remaining. As background: we calculate capacity for candidate assets with something called demand limiting capacity, then divide this by a parameter (
capacity_limit_factor) so that we can appraise candidates with just a portion of the maximum capacity at a time. @tsmbland has suggested just recalculating the demand limiting capacity for every round of appraisal and using this value instead of either the portion of capacity for this tranche (tranche_capacity) or total remaining capacity of asset (according to DLC,remaining_capacity).This resulted in smaller capacities for candidates in the model @ahawkes attached to #1202, so I assume this is working, though we possibly want to double-check.
I was assuming that the DLC would only ever be the smallest value in the NPV case, as for LCOX capacity is included as a variable in the appraisal mini-optimisation and so should be as small as it can be anyway (there's generally a cost to excessive capacity), but there were actually some other cases where it was the smallest when we were using LCOX, which is decidedly fishy. One was for the one agent in @ahawkes's model which was using the LCOX objective type (others were using NPV) and another was for the
two_regionsexample model. I haven't been able to dig down into why that's happening for now.Any ideas off the top of your head @ahawkes?
I'll open this in draft for now so we can discuss.
Fixes #1202.
Type of change
Key checklist
$ cargo test$ cargo docpresent in the previous release
Further checks