fix: the planning_horizons params in CCL need to be string

[yechenyan]

Closes # (if applicable).

Issue:

When using CCL, an error occurs because the year key cannot be found in agg_p_nom_minmax.csv. The error is shown in the image below.

config:

 agg_p_nom_limits:
    agg_offwind: false
    include_existing: false
    file: data/agg_p_nom_minmax.csv

agg_p_nom_minmax.csv:

,,2030,2030,2045,2045,2050,2050
,,min,max,min,max,min,max
country,carrier,,,,,,
DE,onwind,0.1,,180000,181000,0.1,
DE,offwind-all,0.1,,71000,82000,0.1,
DE,solar,0.2,,336000,336500,0.2,
DE,solar rooftop,0.2,,164000,164500,0.2,

Cause:

The issue arises in the function add_CCL_constraints https://github.com/PyPSA/pypsa-eur/blob/master/scripts/solve_network.py#L984.

The parameter planning_horizons should be a string, but a list is passed instead.

This problem was introduced by the changes in the PR for Consistent function scope
1537 (#1537).

Solution:

Modify the definition of the planning_horizons parameter in the rule to be a string.

Changes proposed in this Pull Request

Checklist

• I tested my contribution locally and it works as intended.
• Code and workflow changes are sufficiently documented.
• Changed dependencies are added to envs/environment.yaml.
• Changes in configuration options are added in config/config.default.yaml.
• Changes in configuration options are documented in doc/configtables/*.csv.
• Sources of newly added data are documented in doc/data_sources.rst.
• A release note doc/release_notes.rst is added.

[coroa]

@FabianHofmann It looks like you added a bit of confusion around the planning_horizons argument in the solve_network rule in your #1537 PR.

All functions as far as i can see from a quick skim were only using the snakemake.wildcards.planning_horizons input which is only a single year and you replaced those with snakemake.params.planning_horizons, but the param is mapped to all planning_horizons:

rule solve_sector_network_myopic:
    params:
        [...]
        planning_horizons=config_provider("scenario", "planning_horizons"),

The doc-string you added to prepare_network then talks about a list of years to drive the confusion home.

A solution would be as proposed here to update the params definition, but i'd let you want to make the call on how to disentangle it.

:)

[coroa]

And @yechenyan thanks for the PR in either case.

[FabianHofmann]

@FabianHofmann It looks like you added a bit of confusion around the planning_horizons argument in the solve_network rule in your #1537 PR.

All functions as far as i can see from a quick skim were only using the snakemake.wildcards.planning_horizons input which is only a single year and you replaced those with snakemake.params.planning_horizons, but the param is mapped to all planning_horizons:

rule solve_sector_network_myopic:
    params:
        [...]
        planning_horizons=config_provider("scenario", "planning_horizons"),

The doc-string you added to prepare_network then talks about a list of years to drive the confusion home.

A solution would be as proposed here to update the params definition, but i'd let you want to make the call on how to disentangle it.

:)

lovely, I knew something would come and haunt me (but to my defense it is confusing with wildcards.planning_horizons being plural and having the same name as used in params...). anyway, I am fine with this fix here if that works. @yechenyan thanks for covering. would you mind fixing the docstring as well?

[yechenyan]

I've made the changes and fixed the docstring. @FabianHofmann

[FabianHofmann]

Awesome, after the ci runs through, we can merge

[coroa]

That's incorrect in the myopic runs! The planning_horizons argument to the functions needs to be the currently solving horizon not the last one. That's why the wildcard was being used. EDIT: Not relevant, since this is only done for perfect foresight.

I do not think that -1 is a good default value.

Can you please then also update the doc strings in the function that for perfect foresight a list of planning horizons is passed?

Could also be done as a separate PR, i guess.

[yechenyan]

That's incorrect in the myopic runs! The planning_horizons argument to the functions needs to be the currently solving horizon not the last one. That's why the wildcard was being used. EDIT: Not relevant, since this is only done for perfect foresight.

I do not think that -1 is a good default value.

Can you please then also update the doc strings in the function that for perfect foresight a list of planning horizons is passed?

Could also be done as a separate PR, i guess.

When using perfect forecasts, there is no planning_horizons attribute in the wildcards, so I took planning_horizons[-1] (2050) as the default value. Now, after adding CCL, perfect forecasts no longer result in errors.

Before Consistent function scope 1537 PR, if CCL was added to the configuration, the code would also encounter issues, with the following error(Use the code from three weeks ago: https://github.com/PyPSA/pypsa-eur/tree/590b684afb48c3a3a711caea102dbfb266b2cd0b):

Of course, passing in the planning_horizons list will also result in errors in perfect forecasts.

If needed, I will submit another PR to handle the perfect forecasts.

[coroa]

I think the alternative over in #1560, that i built ontop (so your contributions are counted, @yechenyan ). is more correct, by abolishing the param and using the wildcard consistently instead or None where it is not available. That should give us back what was there before #1537