Cleaning up modify_prenetwork#176
Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull request overview
This PR aims to reduce repeated conversions of snakemake.wildcards.planning_horizons by introducing a single planning_horizon variable and passing it into helper functions in modify_prenetwork.py, aligning function signatures to a consistent pattern.
Changes:
- Updated multiple helper functions to accept
planning_horizonas an explicit argument instead of re-parsing the Snakemake wildcard. - Updated some call sites to pass
planning_horizonthrough (e.g.,first_technology_occurrence,add_wasserstoff_kernnetz,must_run, etc.). - Modified
__main__mock defaults (clusters/run) used for local execution.
Comments suppressed due to low confidence (2)
scripts/pypsa-de/modify_prenetwork.py:1436
new_boiler_bannow requiresplanning_horizon, but the main block still calls it without the new argument. This will raise aTypeErrorwhen running the rule; passplanning_horizonhere (and keep the call sites consistent with the updated function signature).
modify_mobility_demand(n, snakemake.input.modified_mobility_data)
new_boiler_ban(n)
fix_new_boiler_profiles(n)
scripts/pypsa-de/modify_prenetwork.py:1449
unravel_carbonaceous_fuelsnow takesplanning_horizon, but the call site still uses the old signature. This will crash at runtime; passplanning_horizoninto the function call.
unravel_carbonaceous_fuels(n)
unravel_gasbus(n, costs)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1449,7 +1441,7 @@ def limit_cross_border_flows_ac(n, s_max_pu): | |||
|
|
|||
| nuclear_generation_ban(n) | |||
|
|
|||
| first_technology_occurrence(n) | |||
| first_technology_occurrence(n, planning_horizon) | |||
There was a problem hiding this comment.
Fixed in commit 2551927 — coal_generation_ban and nuclear_generation_ban are now called with planning_horizon passed as the second argument.
| clusters=89, | ||
| opts="", | ||
| ll="vopt", | ||
| sector_opts="none", | ||
| planning_horizons="2025", | ||
| run="KN2045_Mix", | ||
| run="endogenous", | ||
| ) | ||
|
|
||
| configure_logging(snakemake) |
There was a problem hiding this comment.
i agree this should be reverted
There was a problem hiding this comment.
Reverted in commit 2551927 — mock_snakemake defaults are back to clusters=27 / run="KN2045_Mix".
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…emake defaults Agent-Logs-Url: https://github.com/PyPSA/pypsa-de/sessions/daa64e18-83c5-443f-8a7e-87596286617a Co-authored-by: lindnemi <56298440+lindnemi@users.noreply.github.com>
|
@copilot apply changes based on the low confidence comments in this thread |
Agent-Logs-Url: https://github.com/PyPSA/pypsa-de/sessions/6935a000-d770-4f18-a69d-43a3884fbf8d Co-authored-by: lindnemi <56298440+lindnemi@users.noreply.github.com>
Fixed in commit 55c63a4 — both |
| @@ -1359,11 +1349,11 @@ def scale_capacity(n, scaling): | |||
| - scaling: A dictionary with scaling limits structured as | |||
| {year: {region: {carrier: limit}}}. | |||
Changes proposed in this Pull Request
Calling
int(snakemake.wildcards.planning_horizonsmultiple times in the rulemodify_prenetwork.Using the convention of
planning_horizonand passing it to functions as input instead of defining internal variables over and over again.I needed to reuse the rule in a different workflow and wanted to hard code the planning horizon which is now much easier/cleaner to do.
Checklist
Required:
Changelog.md. not applicableIf applicable:
scripts/lib/validation. not applicabledoc/*.rstfiles. not applicable