Implement investment limits #1096
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1096 +/- ##
==========================================
+ Coverage 87.42% 87.50% +0.08%
==========================================
Files 55 55
Lines 7379 7507 +128
Branches 7379 7507 +128
==========================================
+ Hits 6451 6569 +118
- Misses 631 637 +6
- Partials 297 301 +4 ☔ 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 implements investment limits for processes using the process_investment_constraints.csv input file that was introduced in PR #1020. Investment limits cap the amount each agent can invest in a process based on annual addition limits, scaled by the agent's share of commodity demand and the number of years elapsed since the previous milestone year.
Changes:
- Added
calculate_investment_limits_for_candidatesfunction to compute per-agent investment limits - Changed
addition_limitfield type fromf64toCapacityfor better type safety - Added
get_annual_addition_limitmethod to support future extension with growth and total capacity limits
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/simulation/investment.rs | Implements the core logic for calculating and applying investment limits to candidate assets, integrates limits into the asset selection process, and adds comprehensive tests |
| src/process.rs | Changes addition_limit type to Capacity and adds get_annual_addition_limit method for future extensibility |
| src/input/process/investment_constraints.rs | Updates type handling throughout to use Capacity instead of f64 and updates all tests accordingly |
| examples/two_regions/process_investment_constraints.csv | Adds example investment constraint data for the two_regions example |
| examples/muse1_default/process_investment_constraints.csv | Adds example investment constraint data for the muse1_default example |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Also, forgot to mention in the description, but I've added investment limits for the And one other thing I've just realised is that these limits won't necessarily be complied with in the circularities algorithm, since we allow capacities to change after the (now constrained) investment stage within |
|
Converting back to draft - some things I want to tidy up |
|
@alexdewar This is getting a bit fiddly. Since investment limits depend on an agent property (namely the agent's It would be so much easier if assets stored the |
Yes I do! There have been a couple of times where I started doing that for other reasons, but ended up not needing it, so never opened a PR. I haven't looked at this PR yet, but we have a couple of options. We could merge as is (possibly adding a warning) or leave this branch here and I can fix it up once we've done the other refactor. Let's talk about it in the meeting. |
|
@alexdewar Sounds good. I've added the constraints for the circularities algorithm in b2eb10d as I wanted this to be complete, albeit a bit messy. If we go down the route of what we've suggested above, then we could just retrieve the investment limit directly for each asset in But, with that in mind, I think this is ready to review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let upper = (1.0 + capacity_margin) * units as f64; | ||
| let mut upper = (1.0 + capacity_margin) * units as f64; | ||
| if let Some(limit) = capacity_limit { | ||
| upper = upper.min(limit.n_units().unwrap() as f64); |
There was a problem hiding this comment.
The use of unwrap() here is safe due to the type compatibility check on lines 676-685, but it would be more robust to use expect() with a descriptive message. This would make debugging easier if the assertion logic somehow changes in the future or if there's an unexpected edge case. Consider changing to: upper = upper.min(limit.n_units().expect("Capacity limit should be discrete here due to compatibility check") as f64);
| upper = upper.min(limit.n_units().unwrap() as f64); | |
| upper = upper.min( | |
| limit | |
| .n_units() | |
| .expect("Capacity limit should be discrete here due to compatibility check") | |
| as f64, | |
| ); |
| fn calculate_investment_limits_for_candidates( | ||
| opt_assets: &[AssetRef], | ||
| commodity_portion: Dimensionless, | ||
| ) -> HashMap<AssetRef, AssetCapacity> { | ||
| // Calculate limits for each candidate asset | ||
| opt_assets | ||
| .iter() | ||
| .filter(|asset| !asset.is_commissioned()) | ||
| .map(|asset| { | ||
| // Start off with the demand-limiting capacity (pre-calculated when creating candidate) | ||
| let mut cap = asset.capacity(); | ||
|
|
||
| // Cap by the addition limits of the process, if specified | ||
| if let Some(limit_capacity) = asset.max_installable_capacity(commodity_portion) { | ||
| cap = cap.min(limit_capacity); | ||
| } | ||
|
|
||
| (asset.clone(), cap) | ||
| }) | ||
| .collect() | ||
| } |
There was a problem hiding this comment.
The new function calculate_investment_limits_for_candidates lacks test coverage. Given that this is a core function implementing the investment limits feature (as mentioned in the PR description), it would be valuable to add unit tests to verify the logic, particularly:
- That it correctly filters only uncommissioned assets
- That it properly combines demand-limiting capacity with investment constraint limits
- That the minimum of the two limits is used when both are present
Description
Since #1020 we have a file allowing users to specify process investment constraints, but these were so far not being used.
In this PR, I've added a function
calculate_investment_limits_for_candidateswhich use values from this input file to cap the amount of investment allowed for each process. These limits are calculated per-agent based on the agent's share of the commodity demand and the number of years elapsed since the previous milestone year (see discussion in #124).The implementation was ultimately quite easy because we already have a
remaining_candidate_capacitymap to cap investments (currently based on demand limiting capacity), so we just needed to modify this map based on the provided investment limits (if any). An alternative would have been to modify thecapacityfield of candidate assets, but I decided against this as this would modify the tranching behaviour which I didn't want.Related things left to do:
Fixes #1069
Type of change
Key checklist
$ cargo test$ cargo docpresent in the previous release
Further checks