Skip to content

Tranching refactor#1391

Merged
tsmbland merged 5 commits into
mainfrom
tranching_refactor
Jul 6, 2026
Merged

Tranching refactor#1391
tsmbland merged 5 commits into
mainfrom
tranching_refactor

Conversation

@tsmbland

@tsmbland tsmbland commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

Description

I've taken some of the main refactoring/small improvements from #1358 and made this a standalone PR.

  • Since Change LCOX to use same optimisation as NPV #1319 we already appraise assets with pre-determined capacities, so can make a number of simplifications to do with that. Firstly, we can set candidate asset capacities to the appropriate capacity, and appraise based on that, rather than passing a max_capacity argument. Also, compare_metric no longer needs to check if capacities are zero, as we can do this beforehand
  • Fix capacity_to_activity validation to disallow 0
  • calculate_investment_limits_for_candidates (now collect_investment_limits_for_candidates) no longer caps capacities based on the original demand limiting capacity (DLC). Effectively, this means that we can now install more than DLC capacity each year. This helps with Problem treating tranches as separate assets #1383 and simplifies the code/model. Note, we're still capping capacities each round by the current DLC, to prevent capacity addition that we definitely don't need (see NPV: Avoid excessive candidate capacity during investment #1271). Debatable whether we want to keep this long term, due to inherent issues with the DLC calculation, and just to simplify things.
  • A couple of example models are affected by the above point.

Fixes # (issue)

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

@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.71429% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.39%. Comparing base (1de6bcb) to head (1de67f5).

Files with missing lines Patch % Lines
src/simulation/investment.rs 85.18% 8 Missing ⚠️
src/simulation/investment/appraisal.rs 83.33% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1391      +/-   ##
==========================================
- Coverage   89.52%   89.39%   -0.14%     
==========================================
  Files          58       58              
  Lines        8361     8352       -9     
  Branches     8361     8352       -9     
==========================================
- Hits         7485     7466      -19     
- Misses        554      564      +10     
  Partials      322      322              

☔ 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.

@tsmbland tsmbland marked this pull request as ready for review July 2, 2026 14:16
Copilot AI review requested due to automatic review settings July 2, 2026 14:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the investment “tranching” flow to appraise assets based on their pre-set capacities (rather than passing a separate max_capacity through appraisal/optimisation), tightens capacity_to_activity validation, and updates regression/example outputs to match the new behaviour.

Changes:

  • Simplifies appraisal/optimisation by removing the max_capacity parameter and using asset.capacity() directly throughout.
  • Updates investment candidate handling and investment-limit collection so annual addition limits are no longer capped by the original demand-limiting capacity (DLC), while still capping per-round capacity by current DLC and remaining addition limits.
  • Disallows capacity_to_activity = 0 and propagates the validation change through schema + release notes, updating regression datasets accordingly.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/data/two_regions/commodity_prices.csv Updated regression output values for prices under the new investment/appraisal behaviour.
tests/data/two_regions/commodity_flows.csv Updated regression output values for flows under the new investment/appraisal behaviour.
tests/data/two_regions/asset_capacities.csv Updated regression output values for installed capacities under the new selection logic.
tests/data/muse1_default/commodity_prices.csv Updated example/regression output values for prices.
tests/data/muse1_default/commodity_flows.csv Updated example/regression output values for flows.
tests/data/muse1_default/asset_capacities.csv Updated example/regression output values for capacities.
src/simulation/investment/appraisal/optimisation.rs Removes max_capacity plumbing and constrains activities based on asset.capacity().
src/simulation/investment/appraisal/constraints.rs Updates activity constraints to derive bounds from the asset’s capacity.
src/simulation/investment/appraisal.rs Removes hypothetical capacity from AppraisalOutput and appraises strictly using the asset’s capacity.
src/simulation/investment.rs Refactors candidate generation + limit collection and adjusts selection/update logic accordingly.
src/output.rs Writes appraisal result capacity from the asset’s capacity (since AppraisalOutput.capacity is removed).
src/input/process.rs Tightens validation to require capacity_to_activity > 0.
src/fixture.rs Updates fixtures to reflect AppraisalOutput no longer carrying a separate capacity field.
schemas/input/processes.yaml Updates schema notes to require capacity_to_activity > 0.
docs/release_notes/upcoming.md Adds release note entry for the capacity_to_activity > 0 validation change.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/simulation/investment/appraisal/constraints.rs Outdated
@tsmbland tsmbland mentioned this pull request Jul 3, 2026
11 tasks
@tsmbland tsmbland force-pushed the tranching_refactor branch from 86aa84c to 1de67f5 Compare July 6, 2026 11:20
@tsmbland

tsmbland commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Also need to merge this one without review. @alexdewar sort of approved these changes in #1358 anyway

@tsmbland tsmbland merged commit 6493671 into main Jul 6, 2026
8 checks passed
@tsmbland tsmbland deleted the tranching_refactor branch July 6, 2026 11:34
@tsmbland tsmbland added this to MUSE Jul 7, 2026
@github-project-automation github-project-automation Bot moved this to ✅ Done in MUSE Jul 7, 2026
@tsmbland tsmbland self-assigned this Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

2 participants