Make all assets divisible

[tsmbland]

Description

This PR follows on from #1480, and effectively makes all assets divisible, in line with the plans described in #1441.

The main change to the code is simplifying AssetCapacity so that now all asset capacities consist of a unit count and unit size. "Non-divisible" assets (i.e. assets who's capacities cannot be broken up) can still exist, but these are now represented like all other assets, just with a unit count of 1 (I've created AssetCapacity::single to make it easier to construct these). There are then a lot of changes to the code to remove special treatment of AssetCapacity::Discrete vs AssetCapacity::Continuous capacities. Hopefully this makes things a lot simpler.

The main functional change (and the reason for doing all this to begin with) is that, whereas previously, assets without a defined process unit_size became non-divisible (represented by AssetCapacity::Continuous), they are now divisible, with a unit size calculated based on demand at the time of investment and the capacity_limit_factor (the "trial capacity" that's referred to in the documentation). This should give more realistic behaviour (in reality you'd never invest in one giant indivisible assets to meet all demands that can only be decommissioned in one go).

For now, I've gone for the approach of strictly enforcing unit sizes, which can lead to slight overinvestment as capacities of units meeting the final bit of demand are not capped (see discussion in the issue). Potential to revisit this later. Keeping the "demand limiting capacity" code for now, even though it's now unused, as it's self-contained and might be useful later.

Other changes:

• renaming n_units to num_units throughout for consistency
• during re-appraisal of existing assets, where we retain one asset at a time, we need to keep track of the original unit count to make sure that we don't retain more units than actually exist. Previously, this was implemented via the same map as process investment constraints, which I didn't like as it mixed distinct concepts, so I've split this into a new map (remaining units). Relevant for tackling Implement calculation for overall addition limit #1428

Appreciate that this is probably a bit tricky to review. I tried to think of ways to break this up but it was ultimately quite difficult.

Fixes #1441

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 Report

❌ Patch coverage is 86.00000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.69%. Comparing base (db5b58e) to head (a91abde).

Files with missing lines	Patch %	Lines
src/simulation/investment.rs	86.00%	7 Missing ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##           assets_n_units    #1483      +/-   ##
==================================================
- Coverage           89.78%   89.69%   -0.09%     
==================================================
  Files                  60       60              
  Lines                8449     8464      +15     
  Branches             8449     8464      +15     
==================================================
+ Hits                 7586     7592       +6     
- Misses                545      554       +9     
  Partials              318      318              

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

Pull request overview

This PR completes the shift to treating all assets as unit-based (divisible) by removing the continuous-capacity representation and standardising dispatch/investment logic around integer unit counts with a defined unit_size. This aligns the simulation/investment model with the “all assets divisible” plan from #1441 and builds on #1480’s num_units support.

Changes:

• Replaced AssetCapacity from a Continuous|Discrete enum to a (num_units, unit_size) representation and updated core asset APIs accordingly.
• Updated investment candidate generation and reappraisal to operate on single-unit assets and track limits via remaining candidate capacity / remaining commissioned units.
• Updated output schema and docs to reflect that num_units is always present and assets are always unit-composed.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/simulation/optimisation/constraints.rs	Treats dispatch capacity variables as unit-count based and scales limits by unit_size.
src/simulation/optimisation.rs	Makes solution capacity iteration and flexible-capacity handling unit-based; updates capacity variable bounds logic.
src/simulation/market.rs	Builds candidate assets as single units, with dynamic unit_size when process lacks one.
src/simulation/investment/appraisal/constraints.rs	Uses asset.total_capacity() for appraisal constraint scaling.
src/simulation/investment/appraisal.rs	Switches appraisal metrics to use total_capacity() and updates tests for unit-based capacities.
src/simulation/investment.rs	Reworks candidate/commissioned appraisal selection around unit counts and capacity limits.
src/simulation.rs	Updates dispatch candidate creation to use the new Asset::new_candidate signature.
src/output.rs	Makes num_units non-optional in output rows and writes it for all assets.
src/model/parameters.rs	Removes validation for candidate_asset_capacity (now needs reintroducing).
src/input/asset.rs	Constructs AssetCapacity via new/single consistently for all assets.
src/fixture.rs	Updates fixtures to use unit-based capacities (including a multi-unit asset fixture).
src/asset/pool.rs	Updates tests/fixtures naming and unit-capacity construction.
src/asset/capacity.rs	Implements new unit-based AssetCapacity struct and updates tests.
src/asset.rs	Removes “divisible vs non-divisible” branching and updates asset creation/capacity APIs for unit-only assets.
schemas/output/asset_capacities.yaml	Updates schema so num_units is always present.
schemas/input/processes.yaml	Updates unit_size documentation to match unit-based model semantics.
schemas/input/assets.yaml	Updates num_units semantics/documentation for determining unit sizing.
docs/model/investment.md	Updates investment documentation to reflect unit-based assets and trial capacity behaviour.

Suppressed comments (1)

src/model/parameters.rs:340

• candidate_asset_capacity is no longer validated. Because Capacity deserialisation allows negative values, a negative (or zero) candidate_asset_capacity can now make the model panic later (e.g. when used as the unit size for dispatch candidates in AssetCapacity::new, which asserts non-negative). This should be rejected during ModelParameters::validate() like before.

        // milestone_years
        check_milestone_years(&self.milestone_years)?;

        // capacity_limit_factor already validated with deserialise_proportion_nonzero

        // fallback_pricing_strategy already validated by deserialisation

        // commodity_balance_epsilon already validated with deserialise_finite_non_negative

        // value_of_lost_load
        check_value_of_lost_load(self.value_of_lost_load)?;

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/simulation/market.rs:450

• A candidate asset is currently constructed with unit_size = 0.0 as a placeholder and then immediately updated. This relies on allowing zero unit sizes and makes it easy to reintroduce divide-by-zero/validation issues later (e.g. if AssetCapacity::new is tightened to require > 0).

            // Create asset with zero capacity, which will be updated below
            let mut asset =
                Asset::new_candidate(process.clone(), region_id.clone(), Capacity(0.0), year)
                    .unwrap();

src/simulation/optimisation.rs:732

• add_capacity_variables divides by unit_size when applying capacity_limits. If a flexible-capacity asset were ever created with unit_size == 0, this becomes a divide-by-zero, and even for valid sizes the limit should be converted to an integer unit bound (otherwise fractional bounds can permit slight limit violations due to floating point rounding).

        let unit_size = asset.capacity().unit_size();
        let current_units = asset.capacity().num_units();

        let lower = (current_units as f64 * (1.0 - capacity_margin)).max(0.0);

        let mut upper = current_units as f64 * (1.0 + capacity_margin);
        if let Some(limit) = capacity_limits.and_then(|limits| limits.get(asset)) {
            upper = upper.min((*limit / unit_size).value());
        }

src/input/asset.rs:141

• This branch constructs AssetCapacity::single(asset.capacity) directly from the CSV capacity. Because AssetCapacity::new/single now assert that unit sizes are non-negative, a negative capacity value in assets.csv would panic before UserAsset::new can return a validation error. Adding an explicit ensure! here keeps invalid input as a recoverable error instead of a crash.

            // Without a process unit_size, lack of num_units implies the asset is indivisible
            // (consists of a single unit).
            AssetCapacity::single(asset.capacity)
        };

[tsmbland]

Hmm this is now giving different results on windows and ubuntu which is unfortunate. Hopefully once #1477 is done this should fix the problem