Skip to content

notebook10 redipatch and market clearing fix - #838

Open
Manish-Khanra wants to merge 1 commit into
mainfrom
Notebook_10_fix
Open

notebook10 redipatch and market clearing fix#838
Manish-Khanra wants to merge 1 commit into
mainfrom
Notebook_10_fix

Conversation

@Manish-Khanra

@Manish-Khanra Manish-Khanra commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

User description

Related Issue

Closes #838

Description

Checklist

  • Documentation updated (docstrings, READMEs, user guides, inline comments, docs 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)

  • Reconfigured the Use Case 3 grid
  • Added a summary table below the redispatch plot listing upward/downward volume per energy source, with a TOTAL row showing up = down.
  • Updated the accompanying markdown explanations.
  • Added a release note entry under Improvements.

PR Type

Enhancement, Bug fix, Documentation


Description

  • Reworks Tutorial 10 redispatch scenario

  • Adds gas peaker clearing behavior

  • Balances redispatch with summary table

  • Updates explanations and release notes


Diagram Walkthrough

flowchart LR
  inputs["Tutorial 10 inputs"]
  clearing["Market clearing cases"]
  redispatch["Redispatch grid setup"]
  summary["Redispatch balance summary"]
  docs["Release notes"]
  inputs -- "adds gas peaker" --> clearing
  inputs -- "updates demand profiles" --> clearing
  inputs -- "repositions units and loads" --> redispatch
  redispatch -- "computes balanced volumes" --> summary
  summary -- "documented in" --> docs
Loading

File Walkthrough

Relevant files
Documentation
release_notes.rst
Document Tutorial 10 redispatch improvements                         

docs/source/release_notes.rst

  • Adds an improvement note for Tutorial 10.
  • Documents the redispatch use case rework.
  • Notes balanced upward/downward redispatch volumes.
  • Mentions the new redispatch summary table.
+1/-0     
Enhancement
10_DSU_and_flexibility.ipynb
Rework clearing and redispatch tutorial scenario                 

examples/notebooks/10_DSU_and_flexibility.ipynb

  • Adds Gas CCGT with natural_gas fuel pricing to create a clearer
    merit-order and peaker-driven clearing price behavior.
  • Revises demand profiles, market-clearing explanations, and pandas fill
    logic for Use Cases 1 and 2.
  • Reconfigures Use Case 3 with northern renewable surplus, southern load
    and dispatchable plants, and removes the DSM unit for
    energy-consistent redispatch.
  • Updates network plot coloring and adds a redispatch volume summary
    table with balanced upward/downward totals.
+183/-60

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

PR Reviewer Guide 🔍

(Review updated until commit dcb7024)

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶

838 - Partially compliant

Compliant requirements:

  • Rework Tutorial 10 redispatch scenario.
  • Add gas peaker / Gas CCGT market-clearing behavior with natural gas pricing.
  • Update demand profiles and clearing explanations.
  • Reconfigure Use Case 3 grid for the redispatch demo.
  • Add a redispatch summary table.
  • Update accompanying notebook markdown explanations.
  • Add a release note entry under Improvements.

Non-compliant requirements:

  • Balance redispatch volumes so total upward volume equals total downward volume in the summary table.
  • Add a redispatch summary table with a TOTAL row that clearly shows upward volume equals downward volume.

Requires further human verification:

  • Verify the notebook runs end-to-end after the scenario changes.
  • Verify the rendered plots and tables visually demonstrate the intended redispatch behavior.
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Summary Bug

The redispatch table sums downward redispatch with clip(upper=0), so downward volumes remain negative. The TOTAL row will therefore show values like upward 100 and downward -100, which does not satisfy the stated requirement that the table show upward volume equals downward volume. Store downward volume as a positive magnitude, and keep the signed net in the separate net column.

"        \"Upward / ramp-up (MWh)\": redisp_p_set.clip(lower=0).sum(),\n",
"        \"Downward / curtailment (MWh)\": redisp_p_set.clip(upper=0).sum(),\n",
"    }\n",
").dropna(subset=[\"Node\"])\n",
"redispatch_summary[\"Net (MWh)\"] = (\n",
"    redispatch_summary[\"Upward / ramp-up (MWh)\"]\n",
"    + redispatch_summary[\"Downward / curtailment (MWh)\"]\n",
")\n",
"redispatch_summary = redispatch_summary.round(1).sort_values(\"Net (MWh)\")\n",
"\n",
"# Totals row: total upward volume == total downward volume -> balanced, no backup\n",
"total_row = pd.DataFrame(\n",
"    {\n",
"        \"Node\": \"\",\n",
"        \"Technology\": \"ALL UNITS\",\n",
"        \"Upward / ramp-up (MWh)\": round(redisp_p_set.clip(lower=0).values.sum(), 1),\n",
"        \"Downward / curtailment (MWh)\": round(\n",
"            redisp_p_set.clip(upper=0).values.sum(), 1\n",
"        ),\n",
"        \"Net (MWh)\": round(redisp_p_set.values.sum(), 1) + 0.0,\n",

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

PR Code Suggestions ✨

Latest suggestions up to dcb7024

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Correct energy volume calculation

redisp_p_set appears to contain power values per snapshot, so summing it directly
labels MW-snapshot totals as MWh and overstates volumes for 15-minute data. Convert
power to energy using the snapshot duration, and report downward curtailment as a
positive volume so the balance check is numerically clear.

examples/notebooks/10_DSU_and_flexibility.ipynb [2959-2985]

+timestep_hours = (
+    redisp_p_set.index.to_series().diff().dt.total_seconds().div(3600).median()
+)
+redisp_energy = redisp_p_set * timestep_hours
+
 redispatch_summary = pd.DataFrame(
     {
         "Node": pp_info["node"],
         "Technology": pp_info["technology"],
-        "Upward / ramp-up (MWh)": redisp_p_set.clip(lower=0).sum(),
-        "Downward / curtailment (MWh)": redisp_p_set.clip(upper=0).sum(),
+        "Upward / ramp-up (MWh)": redisp_energy.clip(lower=0).sum(),
+        "Downward / curtailment (MWh)": -redisp_energy.clip(upper=0).sum(),
     }
 ).dropna(subset=["Node"])
 redispatch_summary["Net (MWh)"] = (
     redispatch_summary["Upward / ramp-up (MWh)"]
-    + redispatch_summary["Downward / curtailment (MWh)"]
+    - redispatch_summary["Downward / curtailment (MWh)"]
 )
 redispatch_summary = redispatch_summary.round(1).sort_values("Net (MWh)")
 
 # Totals row: total upward volume == total downward volume -> balanced, no backup
 total_row = pd.DataFrame(
     {
         "Node": "",
         "Technology": "ALL UNITS",
-        "Upward / ramp-up (MWh)": round(redisp_p_set.clip(lower=0).values.sum(), 1),
+        "Upward / ramp-up (MWh)": round(redisp_energy.clip(lower=0).values.sum(), 1),
         "Downward / curtailment (MWh)": round(
-            redisp_p_set.clip(upper=0).values.sum(), 1
+            -redisp_energy.clip(upper=0).values.sum(), 1
         ),
-        "Net (MWh)": round(redisp_p_set.values.sum(), 1) + 0.0,
+        "Net (MWh)": round(redisp_energy.values.sum(), 1) + 0.0,
     },
     index=["TOTAL"],
 )
Suggestion importance[1-10]: 7

__

Why: The suggestion is relevant because redisp_p_set is likely a power time series, so directly summing it can mislabel MW-snapshot totals as MWh, especially with 15-minute snapshots. Reporting downward curtailment as a positive value also makes the balance check in redispatch_summary clearer, though this mainly affects tutorial/result presentation rather than core simulation logic.

Medium

Previous suggestions

Suggestions up to commit 2c0928c
CategorySuggestion                                                                                                                                    Impact
Possible issue
Correct energy volume calculation

redisp_p_set appears to contain power values, so summing it directly over 15-minute
snapshots reports MW-timesteps rather than MWh. Convert the redispatch time series
to energy using the snapshot duration, and store downward curtailment as a positive
volume so the balance check is numerically meaningful.

examples/notebooks/10_DSU_and_flexibility.ipynb [2959-2985]

+snapshot_hours = redisp_p_set.index.to_series().diff().dt.total_seconds().div(3600)
+snapshot_hours.iloc[0] = snapshot_hours.iloc[1] if len(snapshot_hours) > 1 else 1
+redisp_energy = redisp_p_set.mul(snapshot_hours, axis=0)
+
+upward_mwh = redisp_energy.clip(lower=0).sum()
+downward_mwh = -redisp_energy.clip(upper=0).sum()
+
 redispatch_summary = pd.DataFrame(
     {
         "Node": pp_info["node"],
         "Technology": pp_info["technology"],
-        "Upward / ramp-up (MWh)": redisp_p_set.clip(lower=0).sum(),
-        "Downward / curtailment (MWh)": redisp_p_set.clip(upper=0).sum(),
+        "Upward / ramp-up (MWh)": upward_mwh,
+        "Downward / curtailment (MWh)": downward_mwh,
     }
 ).dropna(subset=["Node"])
 redispatch_summary["Net (MWh)"] = (
     redispatch_summary["Upward / ramp-up (MWh)"]
-    + redispatch_summary["Downward / curtailment (MWh)"]
+    - redispatch_summary["Downward / curtailment (MWh)"]
 )
 redispatch_summary = redispatch_summary.round(1).sort_values("Net (MWh)")
 
 # Totals row: total upward volume == total downward volume -> balanced, no backup
 total_row = pd.DataFrame(
     {
         "Node": "",
         "Technology": "ALL UNITS",
-        "Upward / ramp-up (MWh)": round(redisp_p_set.clip(lower=0).values.sum(), 1),
-        "Downward / curtailment (MWh)": round(
-            redisp_p_set.clip(upper=0).values.sum(), 1
-        ),
-        "Net (MWh)": round(redisp_p_set.values.sum(), 1) + 0.0,
+        "Upward / ramp-up (MWh)": round(upward_mwh.sum(), 1),
+        "Downward / curtailment (MWh)": round(downward_mwh.sum(), 1),
+        "Net (MWh)": round(redisp_energy.values.sum(), 1) + 0.0,
     },
     index=["TOTAL"],
 )
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that summing redisp_p_set directly can mislabel MW-snapshot sums as MWh, especially with 15-minute snapshots. Converting by snapshot duration and reporting downward curtailment as a positive volume improves the correctness and readability of the redispatch summary.

Medium
Preserve expected scenario file

Deleting industrial_dsm_units.csv can break downstream scenario loading if the
loader expects the unit file to exist. Keep the file present but empty, preserving
its columns, so Use Case 3 has no DSM units without introducing a missing-file
failure.

examples/notebooks/10_DSU_and_flexibility.ipynb [2741-2746]

 dsm_path = f"{scenario_path}/industrial_dsm_units.csv"
 if os.path.exists(dsm_path):
-    os.remove(dsm_path)
+    industrial_dsm_units = pd.read_csv(dsm_path)
+    industrial_dsm_units.iloc[0:0].to_csv(dsm_path, index=False)
     print("Steel plant (DSM unit) removed from the Use Case 3 scenario.")
 else:
     print("No DSM unit file present - nothing to remove.")
Suggestion importance[1-10]: 5

__

Why: Keeping an empty industrial_dsm_units.csv is a reasonable robustness improvement if downstream loading expects the file to exist. However, the PR may intentionally rely on missing optional files to exclude DSM units, so the impact is somewhat speculative.

Low
General
Validate scenario remapping

map silently writes NaN for any plant name that is not listed in node_by_name or
max_power_by_name, which can corrupt the scenario inputs. Validate the mapping
before overwriting these required columns so the notebook fails clearly if the input
CSV changes.

examples/notebooks/10_DSU_and_flexibility.ipynb [2689-2690]

+missing_nodes = set(powerplant_units["name"]) - set(node_by_name)
+missing_max_power = set(powerplant_units["name"]) - set(max_power_by_name)
+if missing_nodes or missing_max_power:
+    raise ValueError(
+        "Missing Use Case 3 redispatch configuration for plants: "
+        f"{sorted(missing_nodes | missing_max_power)}"
+    )
+
 powerplant_units["node"] = powerplant_units["name"].map(node_by_name)
 powerplant_units["max_power"] = powerplant_units["name"].map(max_power_by_name)
Suggestion importance[1-10]: 5

__

Why: The suggestion is valid because map would silently introduce NaN values if powerplant_units["name"] changes. Adding validation improves failure clarity, but it is a maintainability safeguard rather than a direct bug fix in the current fixed tutorial data.

Low
Suggestions up to commit c74dacc
CategorySuggestion                                                                                                                                    Impact
Possible issue
Correct energy aggregation

redisp_p_set is power per snapshot, so summing it directly over 15-minute snapshots
reports quarter-hour MW values as MWh and overstates volumes. Convert each snapshot
to energy using the timestep duration, and store downward curtailment as a positive
volume so the table's balance check is unambiguous.

examples/notebooks/10_DSU_and_flexibility.ipynb [2959-2985]

+snapshot_hours = redisp_p_set.index.to_series().diff().dt.total_seconds().div(3600)
+snapshot_hours.iloc[0] = snapshot_hours.iloc[1] if len(snapshot_hours) > 1 else 1
+
+redisp_energy = redisp_p_set.mul(snapshot_hours, axis=0)
+upward = redisp_energy.clip(lower=0).sum()
+downward = -redisp_energy.clip(upper=0).sum()
+
 redispatch_summary = pd.DataFrame(
     {
         "Node": pp_info["node"],
         "Technology": pp_info["technology"],
-        "Upward / ramp-up (MWh)": redisp_p_set.clip(lower=0).sum(),
-        "Downward / curtailment (MWh)": redisp_p_set.clip(upper=0).sum(),
+        "Upward / ramp-up (MWh)": upward,
+        "Downward / curtailment (MWh)": downward,
     }
 ).dropna(subset=["Node"])
 redispatch_summary["Net (MWh)"] = (
     redispatch_summary["Upward / ramp-up (MWh)"]
-    + redispatch_summary["Downward / curtailment (MWh)"]
+    - redispatch_summary["Downward / curtailment (MWh)"]
 )
 redispatch_summary = redispatch_summary.round(1).sort_values("Net (MWh)")
 
 # Totals row: total upward volume == total downward volume -> balanced, no backup
 total_row = pd.DataFrame(
     {
         "Node": "",
         "Technology": "ALL UNITS",
-        "Upward / ramp-up (MWh)": round(redisp_p_set.clip(lower=0).values.sum(), 1),
-        "Downward / curtailment (MWh)": round(
-            redisp_p_set.clip(upper=0).values.sum(), 1
-        ),
-        "Net (MWh)": round(redisp_p_set.values.sum(), 1) + 0.0,
+        "Upward / ramp-up (MWh)": round(upward.sum(), 1),
+        "Downward / curtailment (MWh)": round(downward.sum(), 1),
+        "Net (MWh)": round(redisp_energy.values.sum(), 1) + 0.0,
     },
     index=["TOTAL"],
 )
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that summing redisp_p_set power values can misreport energy volumes as MWh when snapshots are sub-hourly. Converting to energy and making downward curtailment positive improves the accuracy and readability of the redispatch summary table.

Medium
General
Validate scenario mappings

map silently writes NaN for any plant name not present in node_by_name or
max_power_by_name, which can corrupt the scenario CSV and make the later market run
fail in a hard-to-trace way. Validate the mapping result before overwriting
powerplant_units.

examples/notebooks/10_DSU_and_flexibility.ipynb [2689-2690]

-powerplant_units["node"] = powerplant_units["name"].map(node_by_name)
-powerplant_units["max_power"] = powerplant_units["name"].map(max_power_by_name)
+mapped_nodes = powerplant_units["name"].map(node_by_name)
+mapped_max_power = powerplant_units["name"].map(max_power_by_name)
 
+missing_mapping = powerplant_units.loc[
+    mapped_nodes.isna() | mapped_max_power.isna(), "name"
+].tolist()
+if missing_mapping:
+    raise ValueError(
+        f"Missing Use Case 3 node/max_power mapping for: {missing_mapping}"
+    )
+
+powerplant_units["node"] = mapped_nodes
+powerplant_units["max_power"] = mapped_max_power
+
Suggestion importance[1-10]: 5

__

Why: The suggestion is valid because map would silently introduce NaN values for unmapped plant names, which could make the scenario fail later. This is a useful robustness improvement, but the mapping appears intentionally exhaustive for the tutorial data, so the impact is moderate.

Low
Suggestions up to commit c238685
CategorySuggestion                                                                                                                                    Impact
General
Convert power to energy

redisp_p_set contains power values, so summing them directly does not produce MWh;
with 15-minute snapshots this overstates volumes by a factor of four. Convert MW to
energy using the snapshot duration, and make curtailment positive so the balance
check is readable.

examples/notebooks/10_DSU_and_flexibility.ipynb [2959-2985]

+snapshot_hours = redisp_p_set.index.to_series().diff().dt.total_seconds().div(3600)
+snapshot_hours.iloc[0] = (
+    snapshot_hours.dropna().iloc[0] if snapshot_hours.notna().any() else 1.0
+)
+redisp_energy = redisp_p_set.mul(snapshot_hours.to_numpy(), axis=0)
+
+upward = redisp_energy.clip(lower=0).sum()
+downward = -redisp_energy.clip(upper=0).sum()
+
 redispatch_summary = pd.DataFrame(
     {
         "Node": pp_info["node"],
         "Technology": pp_info["technology"],
-        "Upward / ramp-up (MWh)": redisp_p_set.clip(lower=0).sum(),
-        "Downward / curtailment (MWh)": redisp_p_set.clip(upper=0).sum(),
+        "Upward / ramp-up (MWh)": upward,
+        "Downward / curtailment (MWh)": downward,
     }
 ).dropna(subset=["Node"])
 redispatch_summary["Net (MWh)"] = (
     redispatch_summary["Upward / ramp-up (MWh)"]
-    + redispatch_summary["Downward / curtailment (MWh)"]
+    - redispatch_summary["Downward / curtailment (MWh)"]
 )
 redispatch_summary = redispatch_summary.round(1).sort_values("Net (MWh)")
 
 # Totals row: total upward volume == total downward volume -> balanced, no backup
 total_row = pd.DataFrame(
     {
         "Node": "",
         "Technology": "ALL UNITS",
-        "Upward / ramp-up (MWh)": round(redisp_p_set.clip(lower=0).values.sum(), 1),
-        "Downward / curtailment (MWh)": round(
-            redisp_p_set.clip(upper=0).values.sum(), 1
-        ),
-        "Net (MWh)": round(redisp_p_set.values.sum(), 1) + 0.0,
+        "Upward / ramp-up (MWh)": round(upward.sum(), 1),
+        "Downward / curtailment (MWh)": round(downward.sum(), 1),
+        "Net (MWh)": round(redisp_energy.values.sum(), 1) + 0.0,
     },
     index=["TOTAL"],
 )
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that summing redisp_p_set power values directly and labeling them MWh can produce incorrect redispatch volumes, especially for sub-hourly snapshots. Converting by snapshot duration and reporting downward curtailment as a positive volume would make the summary table substantially more accurate and readable.

Medium
Possible issue
Validate required mappings

map() silently writes NaN for any plant name that is not present in node_by_name or
max_power_by_name, which can make the scenario fail later with invalid node or
max_power values. Validate that every powerplant_units["name"] is covered before
overwriting these required columns.

examples/notebooks/10_DSU_and_flexibility.ipynb [2689-2690]

-powerplant_units["node"] = powerplant_units["name"].map(node_by_name)
-powerplant_units["max_power"] = powerplant_units["name"].map(max_power_by_name)
+mapped_nodes = powerplant_units["name"].map(node_by_name)
+mapped_max_power = powerplant_units["name"].map(max_power_by_name)
 
+missing_mappings = powerplant_units.loc[
+    mapped_nodes.isna() | mapped_max_power.isna(), "name"
+].tolist()
+if missing_mappings:
+    raise ValueError(
+        f"Missing Use Case 3 node/max_power mapping for: {missing_mappings}"
+    )
+
+powerplant_units["node"] = mapped_nodes
+powerplant_units["max_power"] = mapped_max_power
+
Suggestion importance[1-10]: 5

__

Why: The suggestion is correct that map() can silently introduce NaN values for unmapped plant names, which would make node or max_power invalid later. The current notebook appears to define mappings for all known plants, so this is mainly a defensive robustness improvement rather than a clear bug fix.

Low
Preserve empty input file

Deleting industrial_dsm_units.csv can break loaders that expect the configured unit
file to exist even when it contains no active units. Keep the file with its schema
and zero rows instead, so Use Case 3 removes the DSM unit without creating a
missing-file failure.

examples/notebooks/10_DSU_and_flexibility.ipynb [2741-2746]

 dsm_path = f"{scenario_path}/industrial_dsm_units.csv"
 if os.path.exists(dsm_path):
-    os.remove(dsm_path)
+    industrial_dsm_units = pd.read_csv(dsm_path).iloc[0:0]
+    industrial_dsm_units.to_csv(dsm_path, index=False)
     print("Steel plant (DSM unit) removed from the Use Case 3 scenario.")
 else:
     print("No DSM unit file present - nothing to remove.")
Suggestion importance[1-10]: 5

__

Why: Keeping industrial_dsm_units.csv with headers and zero rows is a plausible way to avoid missing-file issues while removing active DSM units. However, the PR intentionally deletes the file and the suggestion depends on loader behavior outside the shown diff, so the impact is uncertain.

Low
Suggestions up to commit c238685
CategorySuggestion                                                                                                                                    Impact
General
Correct energy volume totals

The Downward / curtailment (MWh) column is currently negative, so the table does not
show comparable upward and downward volumes despite the text claiming equality. Also
convert redisp_p_set from MW to MWh using the snapshot duration before summing,
otherwise 15-minute data will be overstated.

examples/notebooks/10_DSU_and_flexibility.ipynb [2959-2985]

+snapshot_hours = redisp_p_set.index.to_series().diff().dt.total_seconds().div(3600)
+snapshot_hours.iloc[0] = snapshot_hours.iloc[1] if len(snapshot_hours) > 1 else 1.0
+redisp_energy = redisp_p_set.mul(snapshot_hours, axis=0)
+
+upward = redisp_energy.clip(lower=0).sum()
+downward = -redisp_energy.clip(upper=0).sum()
+
 redispatch_summary = pd.DataFrame(
     {
         "Node": pp_info["node"],
         "Technology": pp_info["technology"],
-        "Upward / ramp-up (MWh)": redisp_p_set.clip(lower=0).sum(),
-        "Downward / curtailment (MWh)": redisp_p_set.clip(upper=0).sum(),
+        "Upward / ramp-up (MWh)": upward,
+        "Downward / curtailment (MWh)": downward,
     }
 ).dropna(subset=["Node"])
 redispatch_summary["Net (MWh)"] = (
     redispatch_summary["Upward / ramp-up (MWh)"]
-    + redispatch_summary["Downward / curtailment (MWh)"]
+    - redispatch_summary["Downward / curtailment (MWh)"]
 )
 redispatch_summary = redispatch_summary.round(1).sort_values("Net (MWh)")
 
 # Totals row: total upward volume == total downward volume -> balanced, no backup
+total_upward = upward.sum()
+total_downward = downward.sum()
 total_row = pd.DataFrame(
     {
         "Node": "",
         "Technology": "ALL UNITS",
-        "Upward / ramp-up (MWh)": round(redisp_p_set.clip(lower=0).values.sum(), 1),
-        "Downward / curtailment (MWh)": round(
-            redisp_p_set.clip(upper=0).values.sum(), 1
-        ),
-        "Net (MWh)": round(redisp_p_set.values.sum(), 1) + 0.0,
+        "Upward / ramp-up (MWh)": round(total_upward, 1),
+        "Downward / curtailment (MWh)": round(total_downward, 1),
+        "Net (MWh)": round(total_upward - total_downward, 1),
     },
     index=["TOTAL"],
 )
Suggestion importance[1-10]: 7

__

Why: This is a valid correctness improvement for the new redispatch_summary: the current downward values are negative despite being described as comparable volumes, and summing redisp_p_set as MWh is questionable if it is MW over sub-hourly snapshots. The impact is meaningful for notebook result accuracy, though it affects explanatory output rather than core functionality.

Medium
Possible issue
Validate scenario remapping

Directly assigning map() results will write NaN into node and max_power for any
plant name not present in the dictionaries. Validate the mapping first so the
notebook fails early instead of producing an invalid scenario CSV.

examples/notebooks/10_DSU_and_flexibility.ipynb [2689-2690]

+missing_units = powerplant_units.loc[
+    ~powerplant_units["name"].isin(node_by_name)
+    | ~powerplant_units["name"].isin(max_power_by_name),
+    "name",
+]
+if not missing_units.empty:
+    raise ValueError(f"Missing Use Case 3 redispatch mapping for units: {missing_units.tolist()}")
+
 powerplant_units["node"] = powerplant_units["name"].map(node_by_name)
 powerplant_units["max_power"] = powerplant_units["name"].map(max_power_by_name)
Suggestion importance[1-10]: 4

__

Why: The suggestion is correct that map() would silently produce NaN for unmapped powerplant_units["name"] values, so early validation would make the notebook more robust. However, the dictionaries in this PR appear to cover the intended units, so this is a defensive maintainability improvement rather than a demonstrated bug.

Low

@Manish-Khanra
Manish-Khanra requested a review from mthede July 13, 2026 19:34
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit c238685

1 similar comment
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit c238685

@codecov

codecov Bot commented Jul 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.35%. Comparing base (b0a4943) to head (dcb7024).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #838   +/-   ##
=======================================
  Coverage   83.35%   83.35%           
=======================================
  Files          56       56           
  Lines        9508     9508           
=======================================
  Hits         7925     7925           
  Misses       1583     1583           
Flag Coverage Δ
pytest 83.35% <ø> (ø)

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

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

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit c74dacc

@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit 2c0928c

@mthede
mthede force-pushed the Notebook_10_fix branch from 2c0928c to dcb7024 Compare July 22, 2026 07:18
@github-actions

Copy link
Copy Markdown

Persistent review updated to latest commit dcb7024

"hour_of_day = time_index.hour + time_index.minute / 60\n",
"daily_shape = 0.5 + 0.5 * np.sin(2 * np.pi * (hour_of_day - 9) / 24)\n",
"demand_values1 = 130 + 280 * daily_shape # north: load centre\n",
"demand_values2 = 20 + 20 * daily_shape # south: mostly generation\n",

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.

Explanation below does not fit to the code ("The demand pattern fluctuates between 44,000 MW and 46,000 MW...")

"### **1. Prepare Input Files**\n",
"\n",
"We'll reuse the DataFrames for powerplant units, demand units, and demand_df that were already created in Use Case 1. These files will be saved in the inputs/tutorial_10 folder for Use Case 3."
"For this redispatch use case we **reconfigure** the two-node grid so that the network congestion produces a clean, easy-to-read result. All renewables are placed in the **north** together with a small local load, while the **south** holds the load centre and all the dispatchable plants (nuclear and gas). The cheap northern renewables are dispatched in full in the day-ahead market, so more power than the 200 MW line can carry has to flow north -> south, congesting the line.\n",

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.

Why do you want to have this dedicated redispatch example in this notebook if the steelplant is not part of it? What additional value do you see here, in comparison to the notebook 11 for showcasing redispatch?

@mthede

mthede commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Other comments not related to changed code:

  • doesn't compile formula: grafik
  • l.1705: meaning unclear: "Transmission grid: Converts the reduced iron into steel."
  • l.1989: might not render correctly? grafik
  • l.2059: simulation throws warnings/errors:
grafik - l.2406: congestion plot may be broken as well? grafik - l.2591: ~ causes strikethrough formatting grafik

@mthede mthede left a comment

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.

see my other comments

@mthede mthede linked an issue Aug 5, 2026 that may be closed by this pull request
@paragpatil39 paragpatil39 self-assigned this Aug 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use correct colors in network plot after pypsa fix

4 participants