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

Add normalize_using_yearly option into hydro function #245

Closed
wants to merge 33 commits into from

Conversation

davide-f
Copy link
Contributor

@davide-f davide-f commented Jun 12, 2022

Closes #244

Change proposed in this Pull Request

This PR aims at adding the normalize_using_yearly option into hydro function

Description

This PR aims at adding the normalize_using_yearly option into hydro function.
To do the above, the output of the runoff function is being postprocess to achieve the desired goal.

Motivation and Context

This feature can be handy to extend the normalization feature that otherwise would be limited to only runoff

How Has This Been Tested?

On my cloud machine

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I tested my contribution locally and it seems to work fine.
  • I locally ran pytest inside the repository and no unexpected problems came up.
  • I have adjusted the docstrings in the code appropriately.
  • I have documented the effects of my code changes in the documentation doc/.
  • I have added newly introduced dependencies to environment.yaml file.
  • I have added a note to release notes doc/release_notes.rst.
  • I have used pre-commit run --all to lint/format/check my contribution

@davide-f
Copy link
Contributor Author

I've proposed a normalization approach, however, I think it is worth discussing how to address countries with no existing hydro production.
Currently, an error is thrown, yet maybe what we may desire is a warning and simply do not normalize such data. what do you think?

@davide-f
Copy link
Contributor Author

davide-f commented Jun 18, 2022

I've proposed a normalization approach, however, I think it is worth discussing how to address countries with no existing hydro production. Currently, an error is thrown, yet maybe what we may desire is a warning and simply do not normalize such data. what do you think?

@euronion Regarding this, hence what to do when we dont have existing buses to scale the country hydro production, we have two options:

  • do not scale it at all
  • scale it using a common factor, that can be calculated using the whole region, è.g. if we are focusing on Africa and we miss existing hydro in Benin, the factor may be: hydro africa TWh/sum(runoff africa). This should work if we have at least an existing plant in the region under interest, if that's not the case, the normalization is not performed. I can add such checks

@euronion
Copy link
Collaborator

Hi @davide-f , thanks for looking into this and your suggestions.

I'm not very familiar with the hydro and runoff functions, but I don't think any of us is. Maybe @coroa has a few comments, ideas and time left to comment on this.

The runoff function follows a quite simple approach and you are right: it is biased by what we are doing in PyPSA-EUR, i.e. normlising using EIA statistics on country level hydro generation.

The hydro function tries to implement a more sophisticated bottom up approach using catchment areas. Before we talk about the messy details there are first a few things I want to clarify:

  1. Do you think it is methodologically correct to normalise the inflow per hydro power plant in hydro? Where would you get the data to normalise with from? Country level data is available from EIA but refers to hydroelectricity generation, not to physical water inflows so there is a mismatch from my PoV between the data.
  2. Or are you thinking about normalising using hydro power plant specific generation data and then normalising the output of the hydro modelling (not the runoff input)?

I agree we should try to avoid terminology which is biased by PyPSA and PyPSA-EUR where we can. So instead of talking about buses we want to talk about the index, i.e. in the case of the hydro function that is probably the specific plant. Would you agree?

Regarding countries without hydro generation:
I don't think I understand the question. Which situation do you mean? Can you maybe give an example of an input for whichi you want to discuss the output?

@davide-f
Copy link
Contributor Author

davide-f commented Jun 20, 2022

The hydro function tries to implement a more sophisticated bottom up approach using catchment areas. Before we talk about the messy details there are first a few things I want to clarify:
...

The proposed normalization procedure resembles the second option that you are stating: first the inflows are calculated using the standard hydro function, then if the option is enabled, instead of returning the standard output, that output is normalized as follows.

Let's consider that a workflow shall calculate the inflows $I_p$ for 10 locations in the set $P_A$ for a country A, but existing hydro powerplants are installed only in 8 set $P^{exist}_A$

$p$ denotes a powerplant location where the inflows are calculated.

Let's assume that for the country (A), where all those 10 buses are located, the total hydro energy production is $E_A$.
Then, each normalized inflow $I^N_p$ is calculated as follows
$I^N_p=I_p\cdot\dfrac{E_A}{\sum_{o \in P^{exist}_A}I_o}$
The same procedure applies for every country.

This procedure is still based on a PyPSA-modelling bias: the output inflow will have a measurement unit in terms of GW/h and losses due to spillage in the dams is not accounted for, yet a multiplying factor could be added.
The major concern for hydro representation is that the hydro function does not model the delays in the water movement as a function of the terrain and slope, and temperature (ice formation), which would be a nice-have; though, it is a first approximation and I guess you are aware of that. Some inspiration and modelling may be derived by Lisflood, yet the model is very complex and strong approximations would be needed to make it easier to handle

I agree we should try to avoid terminology which is biased by PyPSA and PyPSA-EUR where we can. So instead of talking about buses we want to talk about the index, i.e. in the case of the hydro function that is probably the specific plant. Would you agree?

Totally agree

Regarding countries without hydro generation: I don't think I understand the question. Which situation do you mean? Can you maybe give an example of an input for whichi you want to discuss the output?

According to what discussed before, some issues may appear in the normalization procedure if the normalization procedure is used but a country has a non-empty set $P_A$ (hence plants where to calculate the inflows) but no existing powerplants ($P^{exist}_A=\emptyset$). This may lead to problems

@davide-f davide-f marked this pull request as draft June 24, 2022 00:12
@davide-f davide-f marked this pull request as ready for review July 5, 2022 15:04
@davide-f
Copy link
Contributor Author

davide-f commented Jul 5, 2022

This PR is ready for review.
In pypsa-africa we are using this branch and it is ready for review now

@euronion
Copy link
Collaborator

euronion commented Jul 5, 2022

Great! @LukasFrankenQ and I will have a look together.

@energyLS
Copy link
Contributor

energyLS commented Oct 4, 2022

@euronion and @LukasFrankenQ do you have any updates here?

@pz-max
Copy link
Contributor

pz-max commented Nov 1, 2022

Dev meeting: @euronion & @davide-f will talk

@FabianHofmann
Copy link
Contributor

@davide-f sorry that this takes so long. Would it be possible to add a small test for this routine? Then I could review it properly.

@euronion
Copy link
Collaborator

euronion commented Mar 10, 2023

Hi @davide-f ,

I've had a look at it again (finally). I'm thinking of a third option:

  1. Fully remove the normalize_using_yearly from runoff(...) and to not implement it for hydro(...) either.

Reason:

  • The purpose of the function(s) is/would be pure post-processing. They don't require access to the internal atlite data. They would therefore only serve as convinience functions and are not part of the packages core functionality.
  • I don't believe a lot of people actually use the option right now. It is heavily biased towards PyPSA-EUR (it requires the shapes to have an index called countries) and is basically undocumented.
  • The number of possible cases which the convinience function could cover are large - you already did describe a number of different cases which could be covered by the function. This makes the function complicated and difficult to maintain.
  • I think a better solution is to have the users implement their own normalization, based on the data they have and worry about min. number of years, index, weighing, ... themselves.

Basically: I think it's in our interest to keep this repository and packages as lean and focused on its task as possible. Make maintenance easier and the chance of things breaking becomes lower.

What do you think @davide-f and @FabianHofmann ?

@davide-f
Copy link
Contributor Author

@euronion thanks for the revision!
Your comment makes definitely sense.
I had some doubts too. For the time being we can work separately, hence implementing in pypsa earth that post processing.
Then, if the community relies that that code is duplicated may make sense to Integrate.

Based on the experience of pypsa-earth: are you aware of groups using the normalize hydro beyond the pypsa-eur use?

@euronion
Copy link
Collaborator

Based on the experience of pypsa-earth: are you aware of groups using the normalize hydro beyond the pypsa-eur use?

No.

@davide-f
Copy link
Contributor Author

Based on the experience of pypsa-earth: are you aware of groups using the normalize hydro beyond the pypsa-eur use?

No.

Thanks, so I created a new PR with the few changes of this PR that may be worth adding anyway, in my opinion, but feel free to disagree and comment on it.
If we agree, and that PR also works out. This PR can be closed.
I've also modified pypsa-earth to port the changes, see pypsa-meets-earth/pypsa-earth#631

@davide-f davide-f closed this Mar 26, 2023
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.

hydro not compatible with normalize_using_yearly option
6 participants