Fix availabilities validation for non-milestone years by alexdewar · Pull Request #935 · EnergySystemsModellingLab/MUSE2

alexdewar · 2025-10-16T09:23:15Z

Description

Processes are required to have availabilities, flows and parameters defined for every combination of region and milestone year in which they can be commissioned. However, assets in assets.csv are allowed to have a commission_year before the simulation's start, provided it is not before a the process's start_year and if this is the case then it makes validation tricky. You can provide availabilities for every year by putting all in the year column, but if you want to supply entries for individual years instead and the process can be commissioned before the time horizon then you have to provide an entry for all of the possible years, which is horrid (e.g. if the simulation starts in 2020 but your process's start_year is 2000, then you need entries for 2000, 2001, etc.). There's an analogous problem for flows (#934), but I think we need to improve the validation for flows anyway (#931) so I thought it made sense to hold off on tackling it until later.

I originally thought I'd fixed this in #868 but clearly didn't test it properly. Oops.

I verified the fix worked by modifying the simple example like so:

diff --git a/examples/simple/assets.csv b/examples/simple/assets.csv
index 6875fab9..1d1d062c 100644
--- a/examples/simple/assets.csv
+++ b/examples/simple/assets.csv
@@ -1,4 +1,5 @@
 process_id,region_id,agent_id,capacity,commission_year
+GASDRV,GBR,A0_GEX,4002.26,2010
 GASDRV,GBR,A0_GEX,4002.26,2020
 GASPRC,GBR,A0_GPR,3782.13,2020
 WNDFRM,GBR,A0_ELC,3.964844,2020
diff --git a/examples/simple/process_availabilities.csv b/examples/simple/process_availabilities.csv
index dea5736a..6746b727 100644
--- a/examples/simple/process_availabilities.csv
+++ b/examples/simple/process_availabilities.csv
@@ -1,5 +1,8 @@
 process_id,regions,years,time_slice,limit_type,value
-GASDRV,all,all,annual,up,0.9
+GASDRV,all,2010,annual,up,0.9
+GASDRV,all,2020,annual,up,0.9
+GASDRV,all,2030,annual,up,0.9
+GASDRV,all,2040,annual,up,0.9
 GASPRC,all,all,annual,up,0.9
 GASCGT,all,all,annual,up,0.9
 RGASBR,all,all,annual,up,1.0
diff --git a/examples/simple/processes.csv b/examples/simple/processes.csv
index 63f48a97..de5efc64 100644
--- a/examples/simple/processes.csv
+++ b/examples/simple/processes.csv
@@ -1,5 +1,5 @@
 id,description,regions,primary_output,start_year,end_year,capacity_to_activity
-GASDRV,Dry gas extraction,all,GASPRD,2020,2040,1.0
+GASDRV,Dry gas extraction,all,GASPRD,2000,2040,1.0
 GASPRC,Gas processing,all,GASNAT,2020,2040,1.0
 WNDFRM,Wind farm,all,ELCTRI,2020,2040,31.54
 GASCGT,Gas combined cycle turbine,all,ELCTRI,2020,2040,31.54

Do we think it would be worth having one or more examples to exercise this part of the code? We could just have one example that has assets commissioned before the time horizon. We could later add assets commissioned between milestone years and after the time horizon when we've done #859. Thoughts? @tsmbland: this may relate to #655.

Fixes #866.

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

Fixes #866.

Copilot

Pull Request Overview

This PR fixes the validation logic for process availabilities to correctly handle cases where processes can be commissioned before the simulation's milestone years. The fix ensures that only milestone years (years >= base_year) require availability entries, while pre-milestone years are checked lazily when needed.

Split validation into separate functions for milestone years and time slices
Modified validation to only require entries for milestone years, not all process years
Updated function signatures to pass the base_year parameter through the call chain

Reviewed Changes

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

File	Description
src/input/process/availability.rs	Core validation logic split and fixed to handle non-milestone years correctly
src/input/process.rs	Updated to pass base_year parameter to availability reading functions
src/input/process/flow.rs	Minor formatting fix removing trailing comma
src/asset.rs	Improved error message clarity for missing process availabilities

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-16T09:24:20Z

src/input/process/availability.rs

+        .years
+        .iter()
+        .copied()
+        .filter(|&year| year >= base_year);


The filtering logic for milestone years should be documented to clarify why only years >= base_year are considered milestone years, especially since this is a key part of the bug fix.

Copilot · 2025-10-16T09:24:20Z

src/input/process/availability.rs

+                time_slice_info
+                    .iter_ids()
+                    .filter(|ts| !map_for_region_year.contains_key(ts))
+                    .map(|ts| (region_id, year, ts)),


The tuple construction uses year directly, but year is of type &Year from the iterator. This should be *year to dereference the borrowed value for consistency with the tuple type expected.

Suggested change

.map(|ts| (region_id, year, ts)),

.map(|ts| (region_id, *year, ts)),

codecov · 2025-10-16T09:24:41Z

Codecov Report

❌ Patch coverage is 68.85246% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.88%. Comparing base (b80ce22) to head (4704d9c).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
src/input/process/availability.rs	72.72%	9 Missing and 6 partials ⚠️
src/input/process.rs	50.00%	0 Missing and 2 partials ⚠️
src/asset.rs	0.00%	1 Missing ⚠️
src/input/process/flow.rs	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #935      +/-   ##
==========================================
- Coverage   84.93%   84.88%   -0.06%     
==========================================
  Files          50       50              
  Lines        5323     5364      +41     
  Branches     5323     5364      +41     
==========================================
+ Hits         4521     4553      +32     
- Misses        574      579       +5     
- Partials      228      232       +4

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

tsmbland

Looks good!

dalonsoa

This looks good to me.

Just to make sure I understand the implications, in your modified example, as the process start year is 2000 but the asset commission year is 2010, in the old way you would have needed to provide process availabilities for the whole range 2000-2010 + milestones, but now only is needed from 2010, right?

dalonsoa · 2025-10-17T05:17:45Z

src/input/process/availability.rs

    model_dir: &Path,
    processes: &ProcessMap,
    time_slice_info: &TimeSliceInfo,
+    base_year: u32,


Add to the docstring.

Oops! It's a pity there doesn't seem to be any Rust tooling to check these doc comments...

dalonsoa · 2025-10-17T05:18:09Z

src/input/process/availability.rs

    iter: I,
    processes: &ProcessMap,
    time_slice_info: &TimeSliceInfo,
+    base_year: u32,


Same here. This function is missing details in the docstring, anyway.

alexdewar · 2025-10-17T10:27:19Z

Just to make sure I understand the implications, in your modified example, as the process start year is 2000 but the asset commission year is 2010, in the old way you would have needed to provide process availabilities for the whole range 2000-2010 + milestones, but now only is needed from 2010, right?

Not quite. Imagine you have a simulation with milestone years [2020, 2030], a process with start_year 2000 and an asset with a commission year of 2010 (NB: before start of simulation), then:

Previously: you need availabilities for all years 2000-2020 and 2030
Now: you just need them for [2010, 2020, 2030] (i.e. all milestone years plus any other ones before the simulation you actually use)

Eventually we want to be able to include assets commissioned between milestone years and after the simulation too for completeness (#859).

alexdewar added 2 commits October 16, 2025 09:47

Make error message for missing availabilities clearer

c2acd36

Fix validation for availabilities

5c427d1

Fixes #866.

alexdewar requested review from Aurashk, Copilot, dalonsoa and tsmbland October 16, 2025 09:23

Copilot AI reviewed Oct 16, 2025

View reviewed changes

tsmbland mentioned this pull request Oct 16, 2025

[DRAFT] Scripts for building models #937

Closed

10 tasks

tsmbland approved these changes Oct 16, 2025

View reviewed changes

dalonsoa approved these changes Oct 17, 2025

View reviewed changes

Add missing detail to doc comments

4704d9c

alexdewar enabled auto-merge October 17, 2025 10:31

alexdewar merged commit 8482274 into main Oct 17, 2025
8 checks passed

alexdewar deleted the fix-availabilities-required-all-years branch October 17, 2025 10:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix availabilities validation for non-milestone years#935

Fix availabilities validation for non-milestone years#935
alexdewar merged 3 commits intomainfrom
fix-availabilities-required-all-years

alexdewar commented Oct 16, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 16, 2025

Uh oh!

Copilot AI Oct 16, 2025

Uh oh!

codecov bot commented Oct 16, 2025 •

edited

Loading

Uh oh!

tsmbland left a comment

Uh oh!

dalonsoa left a comment

Uh oh!

dalonsoa Oct 17, 2025

Uh oh!

alexdewar Oct 17, 2025

Uh oh!

dalonsoa Oct 17, 2025

Uh oh!

alexdewar commented Oct 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	.map(\|ts\| (region_id, year, ts)),
	.map(\|ts\| (region_id, *year, ts)),

Conversation

alexdewar commented Oct 16, 2025

Description

Type of change

Key checklist

Further checks

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Oct 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tsmbland left a comment

Choose a reason for hiding this comment

Uh oh!

dalonsoa left a comment

Choose a reason for hiding this comment

Uh oh!

dalonsoa Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

alexdewar Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

dalonsoa Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

alexdewar commented Oct 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov bot commented Oct 16, 2025 •

edited

Loading