Skip to content

✨ Allow processes operating in years where they cannot be commission to be in the graph#1000

Merged
tsmbland merged 11 commits intomainfrom
93_graph_validation
Nov 20, 2025
Merged

✨ Allow processes operating in years where they cannot be commission to be in the graph#1000
tsmbland merged 11 commits intomainfrom
93_graph_validation

Conversation

@dalonsoa
Copy link
Copy Markdown
Collaborator

@dalonsoa dalonsoa commented Nov 12, 2025

Description

The way of doing this has been by considering the processes lifetime and using the flow of whatever process that can be operating in the target region and year. Since we are only interested in the flow direction and this does not change, it does not matter what specific flow we use.

No tests have been added, as that module does not contain any tests for any function, but happy to add them.

Fixes #930
Fixes #931

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

Further checks

  • Code is commented, particularly in hard-to-understand areas
  • Tests added that prove fix is effective or that feature works

@dalonsoa dalonsoa requested review from Aurashk and tsmbland November 12, 2025 09:06
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 12, 2025

Codecov Report

❌ Patch coverage is 50.87719% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.02%. Comparing base (8c8509f) to head (951b731).
⚠️ Report is 44 commits behind head on main.

Files with missing lines Patch % Lines
src/graph.rs 33.33% 16 Missing ⚠️
src/cli.rs 16.66% 3 Missing and 2 partials ⚠️
src/output.rs 0.00% 4 Missing ⚠️
src/graph/validate.rs 83.33% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1000      +/-   ##
==========================================
- Coverage   84.26%   84.02%   -0.25%     
==========================================
  Files          52       52              
  Lines        5974     6015      +41     
  Branches     5974     6015      +41     
==========================================
+ Hits         5034     5054      +20     
- Misses        695      714      +19     
- Partials      245      247       +2     

☔ View full report in Codecov by Sentry.
📢 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.

@dalonsoa
Copy link
Copy Markdown
Collaborator Author

I've no idea why the link checker fails. It would seem that the offending link is the RSE Team webpage, but that one most definitely works, so not sure why it complains.

@tsmbland
Copy link
Copy Markdown
Collaborator

I've no idea why the link checker fails. It would seem that the offending link is the RSE Team webpage, but that one most definitely works, so not sure why it complains.

It's been doing this a lot recently... not sure why! Usually it fixes itself if you run it again

@dalonsoa
Copy link
Copy Markdown
Collaborator Author

Ohhhh!!! This is item #1000 !!!

Copy link
Copy Markdown
Collaborator

@tsmbland tsmbland left a comment

Choose a reason for hiding this comment

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

This is good, and will help with solving the investment order because we still need processes to appear even after their end_year (we use the investment algorithm to decide which existing assets to keep, even if new ones can't be invested in).

Worth noting though that this won't have any impact on the graph validation (i.e. the stuff in graphs/validate.rs where we're checking SED/SVD/OTH commodities against the commodity type rules), because this takes process availability into account and there won't be any process availabilities defined after the end_year (and the behaviour in this case is to remove the edge)

I wouldn't suggest touching any of this in this PR, and certainly not until #971 is merged (I had a quick look at this but my impression was that it still need a fair bit of work)

This could all become a bit of a headache, because who knows what the availability limits will be in any particular year - all depends when the assest was commissioned and we can't know this upfront...?

///
/// It considers the lifetime of the process and returns the first flow that can operate in the
/// target year and region. If no flow is found fulfilling that, None is returned.
fn get_flow_for_year(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think probably cleaner for this to take process: &Process

src/graph.rs Outdated
/// Helper function to return a possible flow operating in the requested year
///
/// It considers the lifetime of the process and returns the first flow that can operate in the
/// target year and region. If no flow is found fulfilling that, None is returned.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this needs a bit more explanation/justification - the main point being that graphs only take flow directionality into account, and this is not allowed to change for all years that a process can operate

@tsmbland
Copy link
Copy Markdown
Collaborator

I guess also it's technically possible to have a pre-defined asset (in asset.csv) with a decommission year > process end year + lifetime, but perhaps that's unlikely enough that we can brush that under the rug...

@dalonsoa
Copy link
Copy Markdown
Collaborator Author

Worth noting though that this won't have any impact on the graph validation (i.e. the stuff in graphs/validate.rs where we're checking SED/SVD/OTH commodities against the commodity type rules), because this takes process availability into account and there won't be any process availabilities defined after the end_year (and the behaviour in this case is to remove the edge)

Should we consider in the validation potential process availabilities, as we have done here by accounting for the lifetime? A validation defined that way, will not prevent some scenarios to not be valid if there's no investment in certain processes at the right time, but at least it will discard models that are definitely not feasible upfront.

@tsmbland
Copy link
Copy Markdown
Collaborator

Worth noting though that this won't have any impact on the graph validation (i.e. the stuff in graphs/validate.rs where we're checking SED/SVD/OTH commodities against the commodity type rules), because this takes process availability into account and there won't be any process availabilities defined after the end_year (and the behaviour in this case is to remove the edge)

Should we consider in the validation potential process availabilities, as we have done here by accounting for the lifetime? A validation defined that way, will not prevent some scenarios to not be valid if there's no investment in certain processes at the right time, but at least it will discard models that are definitely not feasible upfront.

Yeah I guess so. So keep the edge if it's possible to have an asset with availability in that year/timeslice. Probably wouldn't be too difficult actually since availabilities are linked to the commission year the same way as lifetime, so it would be similar to what you've done here

@dalonsoa
Copy link
Copy Markdown
Collaborator Author

Worth noting though that this won't have any impact on the graph validation (i.e. the stuff in graphs/validate.rs where we're checking SED/SVD/OTH commodities against the commodity type rules), because this takes process availability into account and there won't be any process availabilities defined after the end_year (and the behaviour in this case is to remove the edge)

Should we consider in the validation potential process availabilities, as we have done here by accounting for the lifetime? A validation defined that way, will not prevent some scenarios to not be valid if there's no investment in certain processes at the right time, but at least it will discard models that are definitely not feasible upfront.

Yeah I guess so. So keep the edge if it's possible to have an asset with availability in that year/timeslice. Probably wouldn't be too difficult actually since availabilities are linked to the commission year the same way as lifetime, so it would be similar to what you've done here

Done! Please, have a look and merge if happy.

@dalonsoa dalonsoa requested a review from tsmbland November 20, 2025 06:08
@tsmbland tsmbland merged commit 6c9773b into main Nov 20, 2025
8 checks passed
@tsmbland tsmbland deleted the 93_graph_validation branch November 20, 2025 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants