Skip to content

Smarter asset grouping - #1492

Draft
tsmbland wants to merge 13 commits into
asset_equalisationfrom
asset_grouping
Draft

Smarter asset grouping#1492
tsmbland wants to merge 13 commits into
asset_equalisationfrom
asset_grouping

Conversation

@tsmbland

@tsmbland tsmbland commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Description

#1477 equalises utilisation of equivalent assets in dispatch, to avoid the solver arbitrarily utilising one over another. There's some ambiguity over what "equivalent" means. For the purposes of that PR, equivalent assets are assets of the same process in the same region. However, that isn't particularly robust:

  • Assets from different processes may have identical parameters for the purposes of dispatch, so should be equalised. (e.g. two processes that differ only in capital cost, which isn't relevant for dispatch)
  • Similarly, assets from the same process may have different parameters (i.e. if the commission year is different), so should not be equalised.

This PR uses a slightly more robust approach of comparing the relevant parameters between assets. Assets in the same region are considered identical if their variable operating costs, flows and availability limits are identical. Comparisons are deliberately conservative such that any difference in any of these, however small, means the assets are considered not equivalent. Adding any kind of tolerance would be much more fiddly. We shouldn't have to worry about floating point differences here as the parameters we're comparing come (almost) directly from the input data.

To avoid lots of potentially expensive pairwise comparisons, the approach is to first create a hash for each asset using the relevant parameters, then only do full comparisons on assets with the same hash (as comparing based on hash alone may create false positives, however unlikely).

Overall, compared with the parent branch, this doesn't seem to have any detectable impact on performance in the models I've tested. Nor does it change any of the example model results, as none of these suffer from process parameters changing over time or redundant processes.

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 Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.01%. Comparing base (35c6a53) to head (eec75d1).

Additional details and impacted files
@@                  Coverage Diff                   @@
##           asset_equalisation    #1492      +/-   ##
======================================================
- Coverage               90.04%   90.01%   -0.04%     
======================================================
  Files                      60       60              
  Lines                    8597     8659      +62     
  Branches                 8597     8659      +62     
======================================================
+ Hits                     7741     7794      +53     
- Misses                    537      545       +8     
- Partials                  319      320       +1     

☔ 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 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 refines the “equal utilisation” dispatch constraints by introducing a more robust notion of dispatch-equivalence between assets, so the optimiser equalises utilisation across assets that behave the same in dispatch (even across different processes), while avoiding equalisation when key dispatch-driving parameters differ.

Changes:

  • Update equal-utilisation constraints to group assets by dispatch-equivalence rather than (region, process) identity.
  • Introduce Asset::is_dispatch_equivalent to centralise the equivalence definition (and add unit tests for it).

Reviewed changes

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

File Description
src/simulation/optimisation/constraints.rs Switch asset grouping strategy for equal-utilisation constraints to use dispatch-equivalence comparisons.
src/asset.rs Add is_dispatch_equivalent helper and tests to define/validate dispatch-equivalence semantics.

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

Comment thread src/simulation/optimisation/constraints.rs Outdated
Comment thread src/asset.rs

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

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

Suppressed comments (1)

src/asset.rs:721

  • is_dispatch_equivalent relies on == for ActivityLimits and the IndexMap of flows. Both are order-sensitive (because they contain/are IndexMaps), so two assets with identical limits/flows but inserted in a different order (e.g. different CSV row ordering, or ActivityLimits::new_from_limits iterating a HashMap for seasonal limits) will be treated as not dispatch-equivalent. That undermines the goal of grouping “equivalent” assets across processes/years.

Consider making the comparisons order-insensitive (while keeping the existing Arc::ptr_eq fast path).

    pub fn is_dispatch_equivalent(&self, other: &Self) -> bool {
        self.region_id == other.region_id
            && (Arc::ptr_eq(&self.activity_limits, &other.activity_limits)
                || self.activity_limits == other.activity_limits)
            && (Arc::ptr_eq(&self.flows, &other.flows) || self.flows == other.flows)

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

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

Suppressed comments (1)

src/asset.rs:683

  • Doc comment grammar: "Equal hashes does not confirm equivalence" should be "Equal hashes do not confirm equivalence".
    /// [`Self::is_dispatch_equivalent`]. Equal hashes does not confirm equivalence, but unequal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants