Modify storage constraint to compute inflows using _profile_aggregate

[abelsiqueira]

This ensures that all profile aggregation is done through that function, which is a requirement for rolling horizon.

Related issues

Closes #1370

Checklist

• I am following the contributing guidelines
• Tests are passing
• Lint workflow is passing
• Docs were updated and workflow is passing

[github-actions]

🤖 CompareMPS report

✅ MPS files match

[github-actions]

Benchmark Results

	a72d144...	f27507f...	a72d144... / f27507f...
energy_problem/create_model	45.2 ± 3.4 s	43.6 ± 7.4 s	1.04 ± 0.19
energy_problem/input_and_constructor	20.3 ± 0.043 s	20.5 ± 0.48 s	0.988 ± 0.023
time_to_load	2.42 ± 0.016 s	2.35 ± 0.0056 s	1.03 ± 0.0071

	a72d144...	f27507f...	a72d144... / f27507f...
energy_problem/create_model	0.223 G allocs: 16.5 GB	0.223 G allocs: 16.5 GB	1
energy_problem/input_and_constructor	0.0475 G allocs: 1.77 GB	0.0476 G allocs: 1.77 GB	0.998
time_to_load	0.151 k allocs: 11.5 kB	0.151 k allocs: 11.5 kB	1

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR.
Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.61%. Comparing base (15907ed) to head (f27507f).
⚠️ Report is 16 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1371      +/-   ##
==========================================
+ Coverage   98.50%   98.61%   +0.11%     
==========================================
  Files          38       38              
  Lines        1404     1372      -32     
==========================================
- Hits         1383     1353      -30     
+ Misses         21       19       -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.

[datejada]

Thanks, @abelsiqueira, it is looking good so far. I would like to use this change to add a test to this constraint. Maybe you can leverage Claude to get it done fast. The test is that it creates the constraints with a setup where we have:

• 4 seasonal storages:
  • 1 with inflows and initial storage level
  • 1 without inflows and initial storage level
  • 1 without inflows but with initial storage level
  • 1 with inflows and without initial storage level
• 1 non-seasonal storage
• 2 milestone years (e.g., 2030 and 2040)
• 2 representative periods per year
• You can have just a week (or a couple of days) in the timeframe to keep it small and simple (check the setup in the Storage Example for the RPs and the timeframe)

The idea is to check that the constraints are the right ones. I hope it is not much of a hassle. It is to cover this edge case that is not currently tested because changes like the ones in this PR will not show any error/problem.

[datejada]

Benchmark Results

a72d144... f27507f... a72d144... / f27507f...
energy_problem/create_model 45.2 ± 3.4 s 43.6 ± 7.4 s 1.04 ± 0.19
energy_problem/input_and_constructor 20.3 ± 0.043 s 20.5 ± 0.48 s 0.988 ± 0.023
time_to_load 2.42 ± 0.016 s 2.35 ± 0.0056 s 1.03 ± 0.0071
a72d144... f27507f... a72d144... / f27507f...
energy_problem/create_model 0.223 G allocs: 16.5 GB 0.223 G allocs: 16.5 GB 1
energy_problem/input_and_constructor 0.0475 G allocs: 1.77 GB 0.0476 G allocs: 1.77 GB 0.998
time_to_load 0.151 k allocs: 11.5 kB 0.151 k allocs: 11.5 kB 1

Benchmark Plots

A plot of the benchmark results have been uploaded as an artifact to the workflow run for this PR. Go to "Actions"->"Benchmark a pull request"->[the most recent run]->"Artifacts" (at the bottom).

Amazing that we gain in speed 😄 even if it is just a bit 😁

[abelsiqueira]

Hi @datejada, indeed it would be good to test these constraints, although it would make sense to have the tests first, then make the change and see that the tests don't break.
I might have some time to implement these tests, but I was actually thinking that maybe I don't need this at all for rolling horizon.

I realize that the inflows are only for the over_clustered_years profiles, and I found a way yesterday to only create rolling parameters for rep_period.
Can you confirm that rolling horizon touches only inside rep_periods? Since there is only one rep_period for rolling horizon, that seems like it.

[datejada]

Hi @datejada, indeed it would be good to test these constraints, although it would make sense to have the tests first, then make the change and see that the tests don't break. I might have some time to implement these tests, but I was actually thinking that maybe I don't need this at all for rolling horizon.

I realize that the inflows are only for the over_clustered_years profiles, and I found a way yesterday to only create rolling parameters for rep_period. Can you confirm that rolling horizon touches only inside rep_periods? Since there is only one rep_period for rolling horizon, that seems like it.

Regarding your comments:

1. Yes, it would be better to add the test first, I have created an issue for it Create new test for storage level over clustering year #1373, so I would blocked this PR until we have the test.
2. Yes, this aggregation is not used in the rolling horizon, as it only meant to be used when we have 1 RP per year. So, I think you can continue the RH without this PR merge, is that correct?

So, I think this is how we should proceed:

• solve Create new test for storage level over clustering year #1373
• rebase this one and test again
• merge if all goes well

What do you think?

[abelsiqueira]

Thanks, @datejada. The plan looks good, except that maybe we don't even need this PR anymore.
Later we can discuss if we still want this modification, for now I'll mark the PR as draft.

[datejada]

@abelsiqueira I am approving this PR to merge it, since I will start working again in Tulipa for the #1082 after the release of v0.5.0 in Tulipa Clsutering. So, I will need a new case study such that the one I mentioned in the comments. I will double-check there constraints. This PR is already an improvement on performance, see benchmark. So, I prefer to start from this the new changes in those constraints.
Thanks!

[abelsiqueira]

Hi @datejada, I've made additional modifications to the constraints in the full rolling horizon PR (#1327). It's not marked as ready to review because of the comment in #1375, but it's otherwise ready for review