Check flows in milestone years only

[dalonsoa]

Description

Removes the need to including flows for every year, modifying the validation checks to ensure they are defined in the milestone years, at least. They might be available in other years should an asset require it, but in milestone years it is a must.

I've checked @alexdewar 's modified example described in the issue, and it passes as planned. Equally, if there's an asset commissioned in a non-milestone year, there is a complain, flagging that.

I still need to add some tests proving this functionality, but first I wanted to check if things were going in the right direction.

Fixes #934

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

[codecov]

Codecov Report

❌ Patch coverage is 88.23529% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.02%. Comparing base (c64a7a8) to head (9136009).
⚠️ Report is 4 commits behind head on main.

Files with missing lines	Patch %	Lines
src/input/process/flow.rs	90.90%	1 Missing and 2 partials ⚠️
src/input/process.rs	0.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #995      +/-   ##
==========================================
+ Coverage   83.94%   84.02%   +0.07%     
==========================================
  Files          52       52              
  Lines        5794     5814      +20     
  Branches     5794     5814      +20     
==========================================
+ Hits         4864     4885      +21     
+ Misses        695      694       -1     
  Partials      235      235              

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

I don't think this is quite right. We don't necessarily need flows for all milestone years, just the milestone years that the process could be commissioned in according to its start_year and end_year. E.g. if the process has an end year of 2030, we don't need flows for 2040 even if 2040 is a milestone year

[dalonsoa]

I don't think this is quite right. We don't necessarily need flows for all milestone years, just the milestone years that the process could be commissioned in according to its start_year and end_year. E.g. if the process has an end year of 2030, we don't need flows for 2040 even if 2040 is a milestone year

Got it! So, the check needs to be done for all milestone years within the process start/end years window, right?

[dalonsoa]

Sorted! And added a couple of tests.

[dalonsoa]

@tsmbland , thinking on #930 and the example described there, I'm not entirely sure that the last modification I made, based on your comment, is correct.

A process might be running in years where it cannot be commissioned, meaning that flows should have a value in those years (in the milestone ones). So, if I understand this right, the last year for which a flow must be provided is end_year + lifetime, not end_year. In this sense, the original code was wrong - as it was considering as the last year end_year, and so is this one, since that part has not changed, specially after your comment.

In summary, flows must exist for all milestone years between start_year and end_year + lifetime, possibly in addition to some other years if an asset requires it.

Am I missing something here?

[tsmbland]

A process might be running in years where it cannot be commissioned, meaning that flows should have a value in those years (in the milestone ones). So, if I understand this right, the last year for which a flow must be provided is end_year + lifetime, not end_year. In this sense, the original code was wrong - as it was considering as the last year end_year, and so is this one, since that part has not changed, specially after your comment.

No that's not right. Flows correspond to the year that an asset was commissioned, i.e. if an asset is commissioned in 2030 it will use the process flows from 2030 all through its lifetime. If the end year for the process is 2030, then we don't need to provide flow data beyond then, even though assets may remain active until later.

What you've done here looks good so I'm going to approve it!

[dalonsoa]

OK, thanks for approving. Merging, now.

But in this case, I do not understand the issue #930 . Will continue the conversation there.