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

Fix NaN values into inflow #242

Merged
merged 6 commits into from
Jun 8, 2022
Merged

Fix NaN values into inflow #242

merged 6 commits into from
Jun 8, 2022

Conversation

davide-f
Copy link
Contributor

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

Closes #241

Change proposed in this Pull Request

This PR aims at avoiding NaN values into the inflow.
Any NaN value would be converted into 0.0 instead before summing to the inflow.

Description

When performing the shift of the inflow dataarray, some NaN values appears hence leading to NaN values into the sum.
This has been fixed.

Motivation and Context

If NaN values remain, this may lead to issues in the usage of the tool

How Has This Been Tested?

Using pypsa-africa

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

@fneum
Copy link
Member

fneum commented Jun 7, 2022

Thanks @davide-f for looking into this! Do you happen to know the reason for the NaN values? Would be good to treat the root cause rather than the symptoms.

atlite/hydro.py Outdated Show resolved Hide resolved
Co-authored-by: Fabian Hofmann <hofmann@fias.uni-frankfurt.de>
@davide-f
Copy link
Contributor Author

davide-f commented Jun 7, 2022

Thanks @davide-f for looking into this! Do you happen to know the reason for the NaN values? Would be good to treat the root cause rather than the symptoms.

The reason is the intrinsic shift operation: when a time series is shifted at the beginning (or end in case of negative shifts) NaN values are created because of missing data.
This link is pretty detailed: https://towardsdatascience.com/all-the-pandas-shift-you-should-know-for-data-analysis-791c1692b5e
There is no circulation in the shift, though it may be an interesting feature

@davide-f
Copy link
Contributor Author

davide-f commented Jun 7, 2022

Thanks @davide-f for looking into this! Do you happen to know the reason for the NaN values? Would be good to treat the root cause rather than the symptoms.

The reason is the intrinsic shift operation: when a time series is shifted at the beginning (or end in case of negative shifts) NaN values are created because of missing data. This link is pretty detailed: https://towardsdatascience.com/all-the-pandas-shift-you-should-know-for-data-analysis-791c1692b5e There is no circulation in the shift, though it may be an interesting feature

Just checked that there is the function "roll" that performs the circular shift, shall we go for it?

@fneum
Copy link
Member

fneum commented Jun 7, 2022

Just checked that there is the function "roll" that performs the circular shift, shall we go for it?

I'd suggest yes.

@davide-f
Copy link
Contributor Author

davide-f commented Jun 7, 2022

Just checked that there is the function "roll" that performs the circular shift, shall we go for it?

I'd suggest yes.

Completely agree (already done), as not using it may lead to underestimate the hydro potential for a region.
Obviously, there may not be temporal correspondence (the flow of one region in 12/2012 will become available in 1/2012 when only 2012 is considered), but I'd say it is a super detail in this case.

This PR is ready. There are no nan fix here, though they were mostly caused by the shift procedure.
I'll be running the script for Africa and let's see. It will take some time however.

@FabianHofmann FabianHofmann merged commit 04d80b6 into PyPSA:master Jun 8, 2022
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.

NaN values into inflows
3 participants