Smarter asset grouping by tsmbland · Pull Request #1492 · EnergySystemsModellingLab/MUSE2

tsmbland · 2026-08-17T11:35:06Z

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.

#1488, now discarded, addressed the second point (at a significant cost to usability), but did not address the first point.

This PR tries to address both points by 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 · 2026-08-17T11:38:06Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.12%. Comparing base (061fbeb) to head (f0f4063).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1492      +/-   ##
==========================================
+ Coverage   90.04%   90.12%   +0.07%     
==========================================
  Files          60       60              
  Lines        8601     8665      +64     
  Branches     8601     8665      +64     
==========================================
+ Hits         7745     7809      +64     
  Misses        537      537              
  Partials      319      319

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

Copilot

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

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

tsmbland · 2026-08-18T08:12:31Z

Hi @ahawkes,

I've gone for a more rigorous approach for grouping assets in the dispatch by checking all the relevant properties. With this approach, assets in the same region are considered equivalent if their variable operating cost, flows and availabilities are identical. The first two mean that there should be no impact on system cost from switching between these assets, and the latter means that equalising utilisation should always be permitted under the availability constraints. This is deliberately conservative - any differences in any of these parameters, however small, means that the assets are considered not equivalent.

I've written the code in such a way that it shouldn't have a great impact on performance

ahawkes · 2026-08-18T10:03:15Z

Sounds good, thanks. I guess you thought about allowing epsilon small differences in the parameters? Probably usually fine, but not when users for some reason choose really small values for things.

…

________________________________ From: Tom Bland ***@***.***> Sent: Tuesday, 18 August 2026 11:12:53 To: EnergySystemsModellingLab/MUSE2 ***@***.***> Cc: Hawkes, Adam D ***@***.***>; Mention ***@***.***> Subject: Re: [EnergySystemsModellingLab/MUSE2] Smarter asset grouping (PR #1492) CAUTION: This message came from outside Imperial. Do not click links or open attachments unless you recognise the sender and were expecting this email. [https://avatars.githubusercontent.com/u/23723407?s=20&v=4]tsmbland left a comment (EnergySystemsModellingLab/MUSE2#1492)<#1492 (comment)> Hi @ahawkes<https://github.com/ahawkes>, I've gone for a more rigorous approach for grouping assets in the dispatch by checking all the relevant properties. With this approach, assets in the same region are considered equivalent if their variable operating cost, flows and availabilities are identical. The first two mean that there should be no impact on system cost from switching between these assets, and the latter means that equalising utilisation should always be permitted under the availability constraints. This is deliberately conservative - any differences in any of these parameters, however small, means that the assets are considered not equivalent. I've written the code in such a way that it shouldn't have a great impact on performance — Reply to this email directly, view it on GitHub<#1492?email_source=notifications&email_token=AC37JLO6JJ65ONZVHIN7KTD5KQFYLA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMZSGU2DINZWGIYKM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5325447620>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AC37JLNJTMBBW4YOEDRSIJD5KQFYLAVCNFSNUABFKJSXA33TNF2G64TZHM3TSMBYGA4TIOJTHNEXG43VMU5TKMJXGEZTSNRYGM42C5QC>. You are receiving this because you were mentioned.Message ID: ***@***.***>

tsmbland · 2026-08-18T10:16:51Z

Sounds good, thanks. I guess you thought about allowing epsilon small differences in the parameters? Probably usually fine, but not when users for some reason choose really small values for things.

Yes I considered this, but any kind of tolerance would complicate things. The current approach uses hashing which is very efficient, but even the smallest differences in parameters will produce a different hash. Any kind of tolerance will mean that we have to do full pairwise comparison between all asset pairs which could be slow (probably minor for most models, but if you have 1000000 assets then that's a lot of comparisons).

So, yes, if a user created otherwise identical assets with variable operating cost 1 and 1.000001, then these would not be grouped together, even though switching between them would have a negligible impact on the objective value. I think that would be a strange thing to do though.

ahawkes · 2026-08-18T10:22:02Z

OK thanks. I guess we only have to do the comparison once, as these parameters don't change after the asset is created.. A

…

________________________________ From: Tom Bland ***@***.***> Sent: 18 August 2026 11:17 To: EnergySystemsModellingLab/MUSE2 ***@***.***> Cc: Hawkes, Adam D ***@***.***>; Mention ***@***.***> Subject: Re: [EnergySystemsModellingLab/MUSE2] Smarter asset grouping (PR #1492) CAUTION: This message came from outside Imperial. Do not click links or open attachments unless you recognise the sender and were expecting this email. [https://avatars.githubusercontent.com/u/23723407?s=20&v=4]tsmbland left a comment (EnergySystemsModellingLab/MUSE2#1492)<#1492 (comment)> Sounds good, thanks. I guess you thought about allowing epsilon small differences in the parameters? Probably usually fine, but not when users for some reason choose really small values for things. Yes I considered this, but any kind of tolerance would complicate things. The current approach uses hashing which is very efficient, but even the smallest differences in parameters will produce a different hash. Any kind of tolerance will mean that we have to do full pairwise comparison between all asset pairs which could be slow (probably minor for most models, but if you have 1000000 assets then that's a lot of comparisons). So, yes, if a user created otherwise identical assets assets with variable operating cost 1 and 1.000001, then these would not be grouped together, even though switching between them would have a negligible impact on the objective value. I think that would be a strange thing to do though. — Reply to this email directly, view it on GitHub<#1492?email_source=notifications&email_token=AC37JLLYVNEYNB2EVEFJWFL5KQUKTA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKMZSGY3TQNRRG42KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-5326786174>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AC37JLMTVZ5TODFMN5ZNKJD5KQUKTAVCNFSNUABFKJSXA33TNF2G64TZHM3TSMBYGA4TIOJTHNEXG43VMU5TKMJXGEZTSNRYGM42C5QC>. You are receiving this because you were mentioned.Message ID: ***@***.***>

dalonsoa

I've a few comments, but otherwise the approach looks solid, so approving.

dalonsoa · 2026-08-18T14:14:52Z

+    /// Hashes are calculated lazily, rather than caching at initialisation, because only assets
+    /// used in dispatch and eligible for equal-utilisation grouping need it. The trade-off is that
+    /// some assets may end up being hashed multiple times if they take part in multiple dispatch
+    /// runs, although the performance cost of this is likely not massive.


Is there any disadvantage on pre-calculating these as, I think, there are not changing over the course of the simulation?

I thought about computing the hash on asset creation (ab2fde7), but decided against this as there are many assets created that never make it into dispatch (e.g. candidate assets), so this would be wasted computation for them.

I'm sure there's a smart middle ground.

dalonsoa · 2026-08-18T14:16:29Z

+    /// used in dispatch and eligible for equal-utilisation grouping need it. The trade-off is that
+    /// some assets may end up being hashed multiple times if they take part in multiple dispatch
+    /// runs, although the performance cost of this is likely not massive.
+    pub(crate) fn dispatch_equivalence_hash(&self) -> u64 {


Why are you being here so specific about the scope of visibility?

Not that important. In principle because it's an implementation detail so doesn't really belong in the public API, but we haven't exactly been intentional about this elsewhere

dalonsoa · 2026-08-18T14:18:04Z

+            .activity_limits
+            .iter_limits()
+            .map(|(selection, limits)| {
+                let mut hasher = DefaultHasher::new();


Why a new hasher here instead of the parent one?

This is to keep it insensitive to order in the activity limits map. We hash each entry with a new hasher, sort the vec of hashes, then hash the sorted hashes with the original hasher

dalonsoa · 2026-08-18T14:18:15Z

+            .flows
+            .values()
+            .map(|flow| {
+                let mut hasher = DefaultHasher::new();


Smarter asset grouping

815b3ec

tsmbland requested a lite review from Copilot August 17, 2026 11:39

Copilot started reviewing on behalf of tsmbland August 17, 2026 11:39 View session

Copilot AI reviewed Aug 17, 2026

View reviewed changes

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

Comment thread src/asset.rs

tsmbland added 4 commits August 17, 2026 13:22

Hash fingerprint

bf9f134

Cache hash

ab2fde7

Add comments

f8a2157

Arc::ptr_eq to avoid deep comparisons

33c9419

tsmbland requested a lite review from Copilot August 17, 2026 12:47

Copilot started reviewing on behalf of tsmbland August 17, 2026 12:48 View session

Copilot AI reviewed Aug 17, 2026

View reviewed changes

tsmbland added 6 commits August 17, 2026 14:14

Add more comments

3e0f666

Revert to lazy hashing

1f0cfeb

Move hashing function

6157e50

Make code more readable

cfb9717

Tests

4859044

Simplify test

83f0d5b

tsmbland requested a lite review from Copilot August 17, 2026 19:58

Copilot started reviewing on behalf of tsmbland August 17, 2026 19:58 View session

Copilot AI reviewed Aug 17, 2026

View reviewed changes

tsmbland added 2 commits August 17, 2026 21:16

Update docs

4a237d1

Fix failing test

eec75d1

tsmbland marked this pull request as ready for review August 18, 2026 08:14

tsmbland requested review from AdrianDAlessandro, dalonsoa and dc2917 August 18, 2026 08:15

tsmbland added this to MUSE Aug 18, 2026

tsmbland self-assigned this Aug 18, 2026

tsmbland moved this to 👀 In review in MUSE Aug 18, 2026

dalonsoa approved these changes Aug 18, 2026

View reviewed changes

Base automatically changed from asset_equalisation to main August 19, 2026 07:06

Merge branch 'main' into asset_grouping

f0f4063

tsmbland enabled auto-merge August 19, 2026 07:07

tsmbland merged commit 8691aa8 into main Aug 19, 2026
8 checks passed

tsmbland deleted the asset_grouping branch August 19, 2026 07:09

github-project-automation Bot moved this from 👀 In review to ✅ Done in MUSE Aug 19, 2026

Uh oh!

Conversation

tsmbland commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Key checklist

Further checks

Uh oh!

codecov Bot commented Aug 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

tsmbland commented Aug 18, 2026

Uh oh!

ahawkes commented Aug 18, 2026 via email

Uh oh!

tsmbland commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ahawkes commented Aug 18, 2026 via email

Uh oh!

dalonsoa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tsmbland commented Aug 17, 2026 •

edited

Loading

codecov Bot commented Aug 17, 2026 •

edited

Loading

tsmbland commented Aug 18, 2026 •

edited

Loading