Skip to content

Allow asset selection to continue with unmet demand#1365

Merged
tsmbland merged 3 commits into
mainfrom
appraisal_fallback
Jun 24, 2026
Merged

Allow asset selection to continue with unmet demand#1365
tsmbland merged 3 commits into
mainfrom
appraisal_fallback

Conversation

@tsmbland

@tsmbland tsmbland commented Jun 23, 2026

Copy link
Copy Markdown
Collaborator

Description

As discussed in #1363, it's possible that, even if asset selection runs out of options, the assets already selected may still be able to meet the full commodity demands in the context of a system dispatch with supply constraints. This at least gives it a chance to try. If it can't, you'll get the warning added here, followed by the usual message about unmet demand from the dispatch, which hopefully should be clear enough.

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 Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.80%. Comparing base (3cb68ed) to head (2bcf85b).

Files with missing lines Patch % Lines
src/simulation/investment.rs 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1365      +/-   ##
==========================================
+ Coverage   89.75%   89.80%   +0.05%     
==========================================
  Files          58       58              
  Lines        8529     8524       -5     
  Branches     8529     8524       -5     
==========================================
  Hits         7655     7655              
+ Misses        561      556       -5     
  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 tsmbland marked this pull request as ready for review June 23, 2026 15:28
Copilot AI review requested due to automatic review settings June 23, 2026 15:28

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 adjusts the investment asset-selection loop so that when appraisal yields no remaining feasible options, the simulation does not abort immediately; instead it logs a warning and proceeds, allowing the subsequent full system dispatch to potentially satisfy demand with already-selected assets (as discussed in #1363 / #1363 context).

Changes:

  • Replaced an early bail! on “no feasible options left” with a warn! and break from the selection loop.
  • Updated logging imports to include warn.

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

Comment thread src/simulation/investment.rs Outdated
Comment on lines 869 to 870
.iter()
.filter(|(_, flow)| **flow > Flow(0.0))
Comment thread src/simulation/investment.rs

@alexdewar alexdewar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

I'm wondering if we still need to log the unmet demand though. When we get to the next dispatch run, either it will succeed, in which case we probably don't care or it will fail, in which case the unmet demand for the dispatch run will be logged anyway (admittedly there may be a discrepancy). What do you think? Another option would be to log the numbers at the debug level to make it less noisy.

We can also just merge this now and tweak it later.

@tsmbland

Copy link
Copy Markdown
Collaborator Author

Another option would be to log the numbers at the debug level to make it less noisy.

Good idea, I'll do that

@tsmbland

Copy link
Copy Markdown
Collaborator Author

Another option would be to log the numbers at the debug level to make it less noisy.

Good idea, I'll do that

Actually I don't think we need this as we can get it from the debug file

@tsmbland tsmbland enabled auto-merge June 24, 2026 08:35
@tsmbland tsmbland added this to MUSE Jun 24, 2026
@tsmbland tsmbland self-assigned this Jun 24, 2026
@tsmbland tsmbland merged commit 76db0a2 into main Jun 24, 2026
8 checks passed
@tsmbland tsmbland deleted the appraisal_fallback branch June 24, 2026 08:40
@github-project-automation github-project-automation Bot moved this to ✅ Done in MUSE Jun 24, 2026
@tsmbland tsmbland mentioned this pull request Jun 25, 2026
11 tasks
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.

3 participants