Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revise TODOs in add_electricity: enable offshore wind and revise hydro #830

Merged
merged 5 commits into from
Aug 17, 2023

Conversation

davide-f
Copy link
Member

@davide-f davide-f commented Aug 12, 2023

Closes #713

Changes proposed in this Pull Request

This PR aims at:

  • re-introducing offshore wind technologies in the workflow
  • revise the hydro implementation to better handle the max_hours definition that was not accurate

This also partially cover #760

Checklist

  • I consent to the release of this PR's code under the AGPLv3 license and non-code contributions under CC0-1.0 and CC-BY-4.0.
  • I tested my contribution locally and it seems to work fine.
  • Code and workflow changes are sufficiently documented.
  • Newly introduced dependencies are added to envs/environment.yaml and doc/requirements.txt.
  • Changes in configuration options are added in all of config.default.yaml and config.tutorial.yaml.
  • Add a test config or line additions to test/ (note tests are changing the config.tutorial.yaml)
  • Changes in configuration options are also documented in doc/configtables/*.csv and line references are adjusted in doc/configuration.rst and doc/tutorial.rst.
  • A note for the release notes doc/release_notes.rst is amended in the format of previous release notes, including reference to the requested PR.

@davide-f davide-f changed the title Revise TODOs in add_electricity Revise TODOs in add_electricity: enable offshore wind and revise hydro Aug 12, 2023
@davide-f davide-f marked this pull request as ready for review August 12, 2023 14:06
@davide-f
Copy link
Member Author

davide-f commented Aug 12, 2023

@hazemakhalek this PR partially handles some issues on hydro: calculate max_hours by country stats. It doesn't fix the problem of placing the same time series on units of the same hydro power plant
FYI: @carlosfv92 , @ekatef

@energyLS this PR should handle the offshore wind ac and dc

@davide-f davide-f modified the milestones: Version 0.3, v0.2.3 Aug 15, 2023
@ekatef
Copy link
Member

ekatef commented Aug 16, 2023

@davide-f amazing work! That is great that you have returned back off-wind :)

Have added some little comments. I also wonder if it'd be possible to use a configuration parameter for the default hydro_max_hours instead of 6 which is hardcoded currently:

hydro_max_hours = hydro.max_hours.where(
hydro.max_hours > 0, hydro.country.map(max_hours_country)
).fillna(6)

Actually, we have PHS_max_hours, but my feeling is that it would be even more handy having to have such a parameter for reservoir plants explicitly declared. What is your feeling about that?

scripts/add_electricity.py Show resolved Hide resolved
scripts/add_electricity.py Show resolved Hide resolved
@@ -568,6 +584,8 @@ def attach_hydro(n, costs, ppl):
hydro_stats["E_store[TWh]"] * 1e3 / hydro_stats["p_nom_discharge[GW]"]
)

max_hours_country.clip(0, inplace=True)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it make sense to specify here lower argument explicitly, as it's done above:

e_target = hydro_stats["E_store[TWh]"].clip(lower=0.2) * 1e6

Copy link
Member Author

@davide-f davide-f Aug 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea!

@davide-f
Copy link
Member Author

@davide-f amazing work! That is great that you have returned back off-wind :)

Have added some little comments. I also wonder if it'd be possible to use a configuration parameter for the default hydro_max_hours instead of 6 which is hardcoded currently:

hydro_max_hours = hydro.max_hours.where(
hydro.max_hours > 0, hydro.country.map(max_hours_country)
).fillna(6)

Actually, we have PHS_max_hours, but my feeling is that it would be even more handy having to have such a parameter for reservoir plants explicitly declared. What is your feeling about that?

Hello :)
All changes are implemented :)

@ekatef
Copy link
Member

ekatef commented Aug 17, 2023

@davide-f amazing work! That is great that you have returned back off-wind :)
Have added some little comments. I also wonder if it'd be possible to use a configuration parameter for the default hydro_max_hours instead of 6 which is hardcoded currently:

hydro_max_hours = hydro.max_hours.where(
hydro.max_hours > 0, hydro.country.map(max_hours_country)
).fillna(6)

Actually, we have PHS_max_hours, but my feeling is that it would be even more handy having to have such a parameter for reservoir plants explicitly declared. What is your feeling about that?

Hello :) All changes are implemented :)

Hello @davide-f :) Added a little additional suggestion. Otherwise looks perfect, I think :)

Great work!

@davide-f
Copy link
Member Author

Added a comment, I think we could go as is, I'd merge, if you disagree, feel free to revise at next occasion :D

@davide-f davide-f merged commit 71ba4c1 into pypsa-meets-earth:main Aug 17, 2023
@davide-f davide-f deleted the fix_offshore branch October 1, 2023 12:26
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.

Important TODOs unhandled in add_electricity
2 participants