New tranching approach

[tsmbland]

Description

Implements the approach suggested in #1358 for selecting tranche sizes, based on a new capacity_granularity parameter in processes.csv. For a "sensible" default I've gone for default_capacity_granularity_factor / capacity_to_activity, where default_capacity_granularity_factor is another new parameter defined in model.toml. I'm relying on this default in all the example models, rather than manually specifying.

I've gone for default_capacity_granularity_factor = 100, which to be honest seems a bit high (if cap2act=1 then the minimum capacity is 100), but this led to a roughly similar number of appraisal rounds compared to before. Making this much smaller would increase running time a lot (not such a problem for these small models), and lead to massive debug files, which is the main reason I've kept it at 100 for now (rather than 10 or 1).

This interacts with the existing divisible assets implementation by making unit_size equal to capacity_granularity for divisible assets (via a method). A new is_divisible parameter controls whether processes are divisible or not. Not much else needed to change here.

Still don't think this is really a complete solution. I quite like being able to toggle granularity vs performance, but I think I agree with Adam that capacities should in some way scale with demand, otherwise you could end up with a huge number of appraisal rounds when demands are high, or an inability to invest in small capacities when demand is low. But, for now, we don't have an agreed upon alternative.

Two regression tests are failing: circularity_npv (which is failing on main) and circularity, both of which are due to #1347, with zero shadow prices being the ultimate culprit (see #1357 (comment))

Fixes #1358

Type of change

• Bug fix (non-breaking change to fix an issue)
• New feature (non-breaking change to add functionality)
• Refactoring (non-breaking, non-functional change to improve maintainability)
• Optimization (non-breaking change to speed up the code)
• Breaking change (whatever its nature)
• Documentation (improve or add documentation)

Key checklist

• All tests pass: $ cargo test
• The documentation builds and looks OK: $ cargo doc
• Update release notes for the latest release if this PR adds a new feature or fixes a bug
  present in the previous release

Further checks

• Code is commented, particularly in hard-to-understand areas
• Tests added that prove fix is effective or that feature works

[tsmbland]

Failing for missing_commodity, two_outputs and circularity. Might be related to #1347, although I wasn't expecting that to crop up with the example models. @alexdewar Let's discuss tomorrow

[tsmbland]

I've had a look at the failing models, and these are all due to commodities having zero/questionable shadow prices in one year, which makes activity coefficients negative the following year:

• circularity: GASNAT in 2030 has a shadow price of zero in summer because there's no demand. In 2040 there is demand for GASNAT in summer (in fact, there's only demand in summer) but the activity coefficients are negative so this doesn't get met

• two_outputs: GASPRD in 2020 has a shadow price of zero in summer because there's excess supply (it's a side product of OAGRSV). Activity coefficients in 2030 are negative. (EDIT: fixed by Allow asset selection to continue with unmet demand #1365)

• missing_commodity: BIOPEL in 2030 has no consumption/production in summer. The shadow price, while not zero (should it be?) is very small, so the coefficients in 2040 are negative. (EDIT: fixed by Allow asset selection to continue with unmet demand #1365)

These all seem like fairly normal scenarios so makes me less convinced that #1347 should be a special feature hidden behind a warning, unless there's a deeper issue here that we can fix.

In the latter two cases, it invests in assets to meet demand in the other seasons, but then exits when it has leftover demand in summer that it's chosen not to meet with these assets. I think an issue with the approach suggested in #1347 is that it would allow the system to invest in extra capacity to meet the demand in summer, whereas really it would have been better to utilise the existing assets in summer all along.

[tsmbland]

@ahawkes I've had a go at implementing the new capacity selection approach as described in #1358.

Two questions/discussion points:

• What do you think would be a "reasonable" value(s) for this parameter? Currently, since I didn't want to manually define it for every process in every model, I've gone for a default of 10 / capacity_to_activity, as I think generally it would scale inversely with capacity_to_activity. The 10 is arbitrary, although we could make this a configurable parameter. Alternatively, we could force users to define capacity_granularity for every process.

• Even with this fix, a few models are still failing, which seems to be related to "weird" shadow prices. See comment above. The solution proposed in Investment fails when only remaining options fail to dispatch #1347 would no doubt allow these models to run to completion, but I think there are some bigger issues we need to think about.

[ahawkes]

@ahawkes I've had a go at implementing the new capacity selection approach as described in #1358.

Two questions/discussion points:

• What do you think would be a "reasonable" value(s) for this parameter? Currently, since I didn't want to manually define it for every process in every model, I've gone for a default of 10 / capacity_to_activity, as I think generally it would scale inversely with capacity_to_activity. The 10 is arbitrary, although we could make this a configurable parameter. Alternatively, we could force users to define capacity_granularity for every process.

Not sure about 10/capacity_to_activity. How about enough capacity to meet about 1/10 of the capacity requirements of the original demand profile - but instead of the DLC approach which focused on the worst time slice (and got capacities that are too large) - we focus on the best (highest availability) time slice - and choose a capacity that can serve 1/10 of the capacity needs in that time slice. There are probably pitfalls to this approach - feel free to refine.

• Even with this fix, a few models are still failing, which seems to be related to "weird" shadow prices. See comment above. The solution proposed in Investment fails when only remaining options fail to dispatch #1347 would no doubt allow these models to run to completion, but I think there are some bigger issues we need to think about.

For this issue, remember we were adding epsilon to make sure things dispatch for the old NPV formulation? I guess this is not similar to that? If yes, then maybe I could take a look at one that is failing to see what's going on?

[tsmbland]

@ahawkes I've had a go at implementing the new capacity selection approach as described in #1358.
Two questions/discussion points:

• What do you think would be a "reasonable" value(s) for this parameter? Currently, since I didn't want to manually define it for every process in every model, I've gone for a default of 10 / capacity_to_activity, as I think generally it would scale inversely with capacity_to_activity. The 10 is arbitrary, although we could make this a configurable parameter. Alternatively, we could force users to define capacity_granularity for every process.

Not sure about 10/capacity_to_activity. How about enough capacity to meet about 1/10 of the capacity requirements of the original demand profile - but instead of the DLC approach which focused on the worst time slice (and got capacities that are too large) - we focus on the best (highest availability) time slice - and choose a capacity that can serve 1/10 of the capacity needs in that time slice. There are probably pitfalls to this approach - feel free to refine.

Ok... that would take us closer to where we were before. The key difference is whether capacities are defined upfront for all years (i.e. in the input data) or whether they depend on run-time info. I assumed you wanted the former so that's how I've done it. In any case, I'd probably lean into one direction or the other, rather than a mixed approach.

• Even with this fix, a few models are still failing, which seems to be related to "weird" shadow prices. See comment above. The solution proposed in Investment fails when only remaining options fail to dispatch #1347 would no doubt allow these models to run to completion, but I think there are some bigger issues we need to think about.

For this issue, remember we were adding epsilon to make sure things dispatch for the old NPV formulation? I guess this is not similar to that? If yes, then maybe I could take a look at one that is failing to see what's going on?

That helps when activity coefficients are zero. In this case, coefficients are negative, so this is a different problem.

[tsmbland]

How about enough capacity to meet about 1/10 of the capacity requirements of the original demand profile - but instead of the DLC approach which focused on the worst time slice (and got capacities that are too large) - we focus on the best (highest availability) time slice - and choose a capacity that can serve 1/10 of the capacity needs in that time slice. There are probably pitfalls to this approach - feel free to refine.

Not sure about this. What if demand is zero (or extremely low) in timeslices with high availability (but may be higher elsewhere). Capacities could end up extremely small, which could mean a huge number of appraisal rounds if there aren't other more feasible technologies. Feels like there could be equally many pitfalls with this approach compared to the DLC approach.

[ahawkes]

How about enough capacity to meet about 1/10 of the capacity requirements of the original demand profile - but instead of the DLC approach which focused on the worst time slice (and got capacities that are too large) - we focus on the best (highest availability) time slice - and choose a capacity that can serve 1/10 of the capacity needs in that time slice. There are probably pitfalls to this approach - feel free to refine.

Not sure about this. What if demand is zero (or extremely low) in timeslices with high availability (but may be higher elsewhere). Capacities could end up extremely small, which could mean a huge number of appraisal rounds if there aren't other more feasible technologies. Feels like there could be equally many pitfalls with this approach compared to the DLC approach.

Yes, fair enough. Though it seems there should be a way to choose based on availabilities, time sliced demands, etc - just needs to be more nuanced than DLC or this alternative demand-based approach. Anyway, in order to have something working, perhaps let's try with 1/cap2act as you suggest and see how it goes? Should we also make it a configurable parameter? If yes, would it be associated with process or commodity? I think I prefer the former.

[tsmbland]

How about enough capacity to meet about 1/10 of the capacity requirements of the original demand profile - but instead of the DLC approach which focused on the worst time slice (and got capacities that are too large) - we focus on the best (highest availability) time slice - and choose a capacity that can serve 1/10 of the capacity needs in that time slice. There are probably pitfalls to this approach - feel free to refine.

Not sure about this. What if demand is zero (or extremely low) in timeslices with high availability (but may be higher elsewhere). Capacities could end up extremely small, which could mean a huge number of appraisal rounds if there aren't other more feasible technologies. Feels like there could be equally many pitfalls with this approach compared to the DLC approach.

Yes, fair enough. Though it seems there should be a way to choose based on availabilities, time sliced demands, etc - just needs to be more nuanced than DLC or this alternative demand-based approach. Anyway, in order to have something working, perhaps let's try with 1/cap2act as you suggest and see how it goes? Should we also make it a configurable parameter? If yes, would it be associated with process or commodity? I think I prefer the former.

As it currently is in this PR:

• users can manually specify capacities at the process level in processes.csv. These are in Capacity units
• if this is unspecified for a process, it will default to x / capacity_to_activity. x in this case has Activity units, currently hardcoded to 10 (completely arbitrary).

I suggest we make the default x configurable in settings.toml (and give this a sensible name!). We can then keep the option to override this at the process level in processes.csv. At the risk of utterly confusing users, it may be better for the process-level field to also take capacities in Activity units (i.e. to then be scaled by capacity_to_activity), rather than absolute Capacity units. This way it's consistent with x (i.e. you define a global default for x, then override it at the process level). Could be really confusing though...

[codecov]

Codecov Report

❌ Patch coverage is 92.70833% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.45%. Comparing base (3cf110a) to head (7571d32).

Files with missing lines	Patch %	Lines
src/input/process.rs	92.30%	1 Missing and 1 partial ⚠️
src/simulation/investment.rs	93.54%	2 Missing ⚠️
src/simulation/investment/appraisal.rs	71.42%	0 Missing and 2 partials ⚠️
src/model/parameters.rs	87.50%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1357      +/-   ##
==========================================
- Coverage   89.68%   86.45%   -3.24%     
==========================================
  Files          58       58              
  Lines        8406     8341      -65     
  Branches     8406     8341      -65     
==========================================
- Hits         7539     7211     -328     
- Misses        550      820     +270     
+ Partials      317      310       -7     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

[alexdewar]

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

Looks like you tell Copilot to ignore certain folders: https://docs.github.com/en/copilot/concepts/context/content-exclusion

Maybe we should do that. Presumably reading all those files eats tokens too!

[alexdewar]

Code looks good as always!

I've got a couple of questions, but happy to approve otherwise. I'm guessing the answer is "yes" to both, but I thought I should double-check!

1. Is @ahawkes happy with this approach, at least for now? We definitely don't want the capacities to be dynamic instead?
2. Do we have a relatively high degree of confidence we can unbreak the failing models? Obvs circularity_npv is failing already, so nothing lost there. I tried dispatching the first non-feasible option for these models (see hack-select-nonfeasible branch) and they still failed.

As Copilot refused to review this, I used local Copilot to do a review and it spotted some things. I've pasted them below. Point 3 seems interesting... do we need to clamp capacity somewhere to avoid exceeding addition limits?

1. Release notes not updated (required by AGENTS.md). docs/release_notes/upcoming.md has no
   entry for this work, but it contains breaking input-format changes that users must act on:

   • processes.csv: unit_size column replaced by capacity_granularity + is_divisible.
   • model.toml: capacity_limit_factor removed; default_capacity_granularity_factor added.

   These belong under "Breaking changes" (and arguably "New features" for is_divisible).

2. Stale documentation in docs/file_formats/input_files.md. The YAML schemas were updated, but
   the markdown reference table was not:

   • Line 21 still documents the removed capacity_limit_factor.
   • Line 234 still documents unit_size (with the old n = ceil(C / U) description) instead of
     capacity_granularity / is_divisible.

   (The unit_size references in docs/release_notes/v2.1.0.md are historical and should be left
   as-is.)

3. Addition limits are no longer strictly enforced — they round up to a multiple of
   capacity_granularity. In update_assets (src/simulation/investment.rs:807-822), a selected
   candidate always contributes a full capacity_granularity, and the remaining-capacity check
   happens after the increment:

   *remaining_capacity = *remaining_capacity - best_asset.capacity();
   if remaining_capacity.total_capacity() <= Capacity(0.0) { /* remove */ }

   For an indivisible process with addition_limit = 250 and capacity_granularity = 100, the
   candidate gets selected three times (remaining: 150 → 50 → −50), investing 300 total — exceeding
   the limit by nearly a full granularity. Previously max_installable_capacity (a min cap)
   bounded this. If exceeding the configured addition_limit is not acceptable, the loop should
   skip/clamp the final tranche when it would overshoot. Worth confirming this is intended.

[alexdewar]

Good spot!

[alexdewar]

Perhaps this should be listed as a bug fix in the release notes?

[alexdewar]

Alternatively, you could have it default to false:

    #[serde(default)]
    is_divisible: bool,

[alexdewar]

A lot of the checks look like this. I've wondered if we should write a macro for it.

[alexdewar]

Another way of doing this would be to unwrap the metrics together, e.g.:

let (metric1, metric2) = self.metric.as_ref().zip(other.metric.as_ref()).expect("Cannot compare non-valid outputs");

Then you wouldn't need to unwrap() below.

[alexdewar]

Itertools lets you do this:

Suggested change

	.collect::<Vec<_>>();
	.collect_vec();

[alexdewar]

Not really related to this PR, but I've been wondering whether we should convert Candidate assets to Selected here rather than in select_best_assets. What do you think?