Skip to content

Rolling horizon dsm#804

Open
Manish-Khanra wants to merge 17 commits into
mainfrom
rolling_horizon_dsm
Open

Rolling horizon dsm#804
Manish-Khanra wants to merge 17 commits into
mainfrom
rolling_horizon_dsm

Conversation

@Manish-Khanra

@Manish-Khanra Manish-Khanra commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Related Issue

Closes #804
If no issue exists, delete this line.

Description

This PR implements rolling-horizon optimization for the DSM units, enabling reactive bidding while maintaining inter-temporal feasibility. Previously, DSM units (steel plants, buildings, hydrogen plants, steam generation plants) optimized over the full simulation horizon. This feature allows units to re-optimize shorter look-ahead windows after each market round while carrying forward component states (e.g., storage SoC, operational status).

Checklist

  • Documentation updated (docstrings, READMEs, user guides, inline comments, doc folder updates etc.)
  • New unit/integration tests added (if applicable)
  • Changes noted in release notes (if any)
  • Consent to release this PR's code under the GNU Affero General Public License v3.0

Additional Notes (optional)

Key Features

  • Rolling-Horizon Mode: Configure via dsm_optimisation_config with horizon_mode, look_ahead_horizon, commit_horizon, and rolling_step keys
  • Three Steel Plant Strategies: cost-optimized (default), profile-guided (tracks normalized load profile), and min-demand (enforces hourly minimums)
  • Fully Extensible: Removed hardcoded technology-specific logic; new DSM unit types can enable rolling-horizon by setting 3 class attributes and optionally 1 method override
  • Production-Ready: All 53 tests passing; backward compatible; comprehensive documentation

@codecov

codecov Bot commented Apr 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 76.00000% with 108 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.09%. Comparing base (6f2857a) to head (ae32acd).

Files with missing lines Patch % Lines
assume/units/dsm_load_shift.py 74.26% 87 Missing ⚠️
assume/units/steel_plant.py 82.00% 9 Missing ⚠️
assume/strategies/naive_strategies.py 58.33% 5 Missing ⚠️
assume/scenario/loader_csv.py 82.60% 4 Missing ⚠️
assume/common/forecaster.py 88.88% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #804      +/-   ##
==========================================
+ Coverage   82.09%   83.09%   +1.00%     
==========================================
  Files          56       56              
  Lines        9081     9518     +437     
==========================================
+ Hits         7455     7909     +454     
+ Misses       1626     1609      -17     
Flag Coverage Δ
pytest 83.09% <76.00%> (+1.00%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@maurerle maurerle self-requested a review April 30, 2026 13:09

@maurerle maurerle 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.

I did not review the dsm_load_shift.py - but all other files.
There are still a few things to do.

I guess that the way in which units are added to the DSMFlex is not that good, as the code seems to not know which parameters of the class exist.
The result is the need for various hasattr checks, as you never know if a param exists or is initialized.
I suggest using planning mode and a larger context window when using an LLM to help with this..

Comment thread assume/strategies/naive_strategies.py Outdated
Comment thread assume/strategies/naive_strategies.py Outdated
Comment thread assume/strategies/naive_strategies.py
Comment thread assume/units/steel_plant.py Outdated
Comment thread assume/units/steel_plant.py Outdated
Comment thread examples/examples.py Outdated
Comment thread tests/test_steel_plant.py Outdated

# Set the congestion_indicator in the instance
# Delete the old component if it exists to avoid Pyomo warnings
if hasattr(instance, "congestion_indicator"):

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.

I don't like this check - can't we test for something like instance.flexibility_measure == "congestion_management_flexibility" in which case the congestion_indicator is set?
This is more like a general thing

Co-authored-by: Copilot <copilot@github.com>
@Manish-Khanra Manish-Khanra requested a review from maurerle April 30, 2026 20:26
@maurerle

Copy link
Copy Markdown
Member

@Manish-Khanra please rebase/merge and fix the conflicts - then this can be merged :)

@Manish-Khanra

Manish-Khanra commented May 18, 2026

Copy link
Copy Markdown
Contributor Author

@Manish-Khanra please rebase/merge and fix the conflicts - then this can be merged :)
@maurerle Has been successfully rebased

Comment thread assume/units/steel_plant.py Outdated
demand_value = self.steel_demand
if "_remaining_demand" in self.components:
demand_value = self.components["_remaining_demand"]
if hasattr(self, "_rh_window_remaining_demand"):

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.

why not initialize _rh_window_remaining_demand in init instead of using hasattr here?

@maurerle maurerle 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.

Hi @Manish-Khanra I reviewed this PR with @reinecfi
we have some recommendations and changes requested.

If something is not easily understood, we can have a call as well.

Comment thread assume/common/forecast_algorithms.py Outdated
Comment thread assume/scenario/loader_csv.py Outdated
return "price_from_cleared_history"

# Otherwise treat as algorithm name and return as-is
return value

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.

Shouldn't this happen automagically?
Via forecaster _registries:

congestion_signal_update_algorithm = self._registries["update"].get(

I guess we currently can only decide if we want to use a self calculated forecast or the existing from forecasts_df.csv
This is still the case with this function (as forecasts_df is always preferred).

I wonder if this function is actually needed. @reinecfi

Comment thread assume/common/forecast_algorithms.py Outdated
Comment thread assume/scenario/loader_csv.py Outdated
Comment thread assume/scenario/loader_csv.py Outdated
Comment on lines +780 to +782
market_prices=initial_market_prices
if initial_market_prices
else unit.get("market_prices"),

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.

Suggested change
market_prices=initial_market_prices
if initial_market_prices
else unit.get("market_prices"),
market_prices=initial_market_prices or unit.get("market_prices"),

what is market_prices here?

Comment thread assume/scenario/loader_csv.py Outdated

if isinstance(price_update_resolved, pd.Series):
# Direct column: use as initial market prices
initial_market_prices["EOM"] = price_update_resolved

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.

hardcoding "EOM" here does not look right?

But I guess this is consistently weird with the DSM unit and DSM forecaster as well.

unit.optimisation_counter = 1
if unit.horizon_mode == "rolling_horizon":
current_market_time = product_tuples[0][0]
did_reoptimize = unit._check_and_reoptimize_rolling_window(

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.

This should be calling update_forecasts_if_needed instead.
This logic should then go into this update_forecasts_if_needed function.

But I think that we need to specify this further together with @reinecfi

Comment thread assume/units/steel_plant.py
Comment thread assume/units/dsm_load_shift.py Outdated
Manish-Khanra and others added 3 commits May 22, 2026 17:23
This is still very different from other units and includes many workarounds.
@Manish-Khanra Manish-Khanra requested a review from maurerle May 23, 2026 12:23
Comment thread assume/common/forecast_algorithms.py Outdated
Returns:
dict: Updated forecast with adaptive prices learned from clearing history.
"""
unit = kwargs.get("unit")

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.

where is this unit set when running this in the simulation (not in the test)

@Manish-Khanra Manish-Khanra requested a review from maurerle June 1, 2026 19:08

@maurerle maurerle 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.

Did you test that everything works well with your latest commit as intended?
I think that some relevant parts were removed there..? For example adding an "adaptive" forecast algorithm function.

The general idea is to keep the rolling horizon out of the loader_csv and implement the update function in the forecast_algorithm instead of the loader_csv so that it can be reused?

But maybe I am mistaken and this is intended?

Comment thread assume/scenario/loader_csv.py Outdated
@Manish-Khanra

Copy link
Copy Markdown
Contributor Author

Did you test that everything works well with your latest commit as intended? I think that some relevant parts were removed there..? For example adding an "adaptive" forecast algorithm function.

The general idea is to keep the rolling horizon out of the loader_csv and implement the update function in the forecast_algorithm instead of the loader_csv so that it can be reused?

But maybe I am mistaken and this is intended?

@maurerle Yes removing the adaptive forecast algorithm was intentional. And you're right that it's a relevant piece, which is exactly why I am moving it to its own PR rather than dropping it. I originally added the adaptive update algorithm in the same commit as rolling horizon, but they are really two separate features. The adaptive algorithm is a forecast update mechanism that refreshes the price forecast for future use. Since rolling horizon doesnot depend on it, I have stashed the forecast algorithm to keep this PR focused on the rolling-horizon implementation and I will submit it as a dedicated PR.

@Manish-Khanra

Copy link
Copy Markdown
Contributor Author

@maurerle The 3.14 checks fail because pypsa <= 0.35.2 (pinned in #808, see #807) becasue the network extra cant install on 3.14. Until the pin can be dropped, I have guarded the notebook 10 with pypsa + network test steps with if: matrix.python-version != '3.14'; 3.10/3.12 are unchanged. This notebook has its own fix now bu I dont know if this temporarily fix like this is appropriate. If not, could you please handle this part?

@Manish-Khanra Manish-Khanra requested a review from maurerle June 3, 2026 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants