Fix get_demand_limiting_capacity to account for seasonal/annual activity limits

[tsmbland]

Description

Part of the problem were facing in #1319 is due to the fact that we're incorrectly calculating demand limiting capacities for processes with seasonal/annual availability limits, which prevents installing enough capacity to meet demand.

For example, wind turbines in the muse1_default model have an annual availability limit of 0.4, which currently isn't taken into account when calculating DLC

Since different processes can have availability limits defined at different levels, which isn't known at the point of calculating DLC, the easiest approach I could think of was to calculate DLC at every possible level (timeslice/season/annual) and take the max.

Ultimately I'm hoping that if we go ahead with plans to change the way tranching works then we may not need this function any more.

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 Report

❌ Patch coverage is 94.82759% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.68%. Comparing base (c2c6833) to head (8075d34).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/time_slice.rs	81.25%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1326      +/-   ##
==========================================
+ Coverage   88.65%   88.68%   +0.02%     
==========================================
  Files          58       58              
  Lines        8551     8599      +48     
  Branches     8551     8599      +48     
==========================================
+ Hits         7581     7626      +45     
- Misses        657      660       +3     
  Partials      313      313              

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

Would have to change if we go ahead with #1329

Something like

/// Get the maximum required capacity across time slices.
fn get_demand_limiting_capacity(
    time_slice_info: &TimeSliceInfo,
    asset: &Asset,
    commodity: &Commodity,
    demand: &DemandMap,
) -> Capacity {
    let coeff = asset.get_flow(&commodity.id).unwrap().coeff;
    let mut capacity = Capacity(0.0);
    let mut demand_cache: HashMap<_, Flow> = HashMap::new();

    // Calculate demand-limiting capacity at each timeslice level and take the max.
    // This is necessary because process availability limits at the seasonal/annual level may
    // necessitate higher capacity than activity limits at finer timeslice levels.
    for level in TimeSliceLevel::iter() {
        for selection in time_slice_info.iter_selections_at_level(level) {
            // Maximum supply within this selection according to the asset's activity limits.
            let max_supply_for_selection = *asset
                .get_activity_per_capacity_limits_for_selection(&selection)
                .end()
                * coeff;

            // Selections with zero supply would imply infinite demand-limiting capacity,
            // so they do not contribute to the maximum.
            if max_supply_for_selection == FlowPerCapacity(0.0) {
                continue;
            }

            // Serviceable demand within this selection.
            //
            // Demand is stored at the commodity balance level. Demand from a balance
            // bucket contributes if:
            //   1. The bucket is contained within this selection, and
            //   2. The asset can operate in at least one constituent timeslice
            //      within that bucket.
            //
            // This reflects the fact that demand is fungible within a balance
            // bucket but not across balance buckets.
            let serviceable_demand_for_selection = *demand_cache
                .entry(selection.clone())
                .or_insert_with(|| {
                    selection
                        .iter_at_level(time_slice_info, commodity.time_slice_level)
                        .unwrap()
                        .filter(|(bucket, _)| {
                            bucket.iter(time_slice_info).any(|(time_slice, _)| {
                                *asset
                                    .get_activity_per_capacity_limits(time_slice)
                                    .end()
                                    > ActivityPerCapacity(0.0)
                            })
                        })
                        .map(|(bucket, _)| demand[&bucket])
                        .sum()
                });

            // Calculate demand-limiting capacity for this selection and take the
            // maximum across all selections.
            capacity = capacity.max(
                serviceable_demand_for_selection / max_supply_for_selection,
            );
        }
    }

    capacity
}

[Copilot]

Pull request overview

This PR fixes demand-limiting capacity (DLC) estimation during investment by accounting for activity/availability limits specified at coarser temporal aggregations (seasonal/annual), preventing under-installation for technologies like wind with annual availability constraints.

Changes:

• Reworked get_demand_limiting_capacity to compute DLC across all TimeSliceLevels and take the maximum implied capacity, using activity limits at the corresponding selection level.
• Added TimeSliceSelection::containing_selection_at_level and extended TimeSliceLevel ordering traits to support level comparisons.
• Updated regression test golden CSV outputs for two_regions and muse1_default, and added a targeted unit test for coarser (annual) limits affecting DLC.

Reviewed changes

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

Show a summary per file

File	Description
tests/data/two_regions/commodity_prices.csv	Updated expected commodity prices to reflect changed investment/dispatch outcomes after DLC fix.
tests/data/two_regions/commodity_flows.csv	Updated expected commodity flows for the two_regions regression case.
tests/data/two_regions/assets.csv	Updated expected commissioned assets list for two_regions.
tests/data/two_regions/asset_capacities.csv	Updated expected installed capacities for two_regions.
tests/data/muse1_default/commodity_prices.csv	Updated expected commodity prices for muse1_default after DLC fix.
tests/data/muse1_default/commodity_flows.csv	Updated expected commodity flows for muse1_default.
tests/data/muse1_default/assets.csv	Updated expected commissioned assets list for muse1_default.
tests/data/muse1_default/asset_capacities.csv	Updated expected installed capacities for muse1_default.
src/time_slice.rs	Added helper to lift a selection to a coarser level; strengthened TimeSliceLevel ordering traits.
src/simulation/investment.rs	Implemented DLC calculation across time-slice levels; added unit test covering annual-limit-driven capacity increases.
src/asset.rs	Added helper to query activity-per-capacity limits for an arbitrary TimeSliceSelection.

---

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

[alexdewar]

I agree with Copilot: I'd return None if the input is invalid instead of panicking. I know it won't panic in the one place this function's actually being used, but I think it's clearer if you have to explicitly unwrap() it at the call site.

[tsmbland]

@alexdewar We may delete this eventually, but I guess it may as well be correct for now

[alexdewar]

Looks good! Think it's worth having the fix even if we'll do away with the code anyway.

[alexdewar]

I agree with Copilot: I'd return None if the input is invalid instead of panicking. I know it won't panic in the one place this function's actually being used, but I think it's clearer if you have to explicitly unwrap() it at the call site.