Always shift solar influx by 30 minutes

zoltanmaric commented 1 year ago

Always Shift Solar Influx by 30 Minutes

Description

Fixes a bug in solar position time shift for ERA5 influx data where for some cases the frequency of the data would be incorrectly inferred. The change always shifts by -30 minutes, regardless of the sampling of the data. See discussion in https://github.com/PyPSA/atlite/issues/256#issuecomment-1268473262 for reasoning.

Motivation and Context

Closes #256

How Has This Been Tested?

The first commit in the pull request introduces a failing unit test that demonstrates the bug reported in #256.
The second commit fixes the bug by always shifting by 30 minutes (removing the inference of the frequency of the data)

Type of change

[x] 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

[x] I tested my contribution locally and it seems to work fine.
[x] I locally ran pytest inside the repository and no unexpected problems came up.
[x] 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.
[x] I have added a note to release notes doc/release_notes.rst.
[x] I have used pre-commit run --all to lint/format/check my contribution

zoltanmaric commented 1 year ago

FYI @euronion @FabianHofmann

zoltanmaric commented 1 year ago

@zoltanmaric this looks great! Thank you for raising and tackling this issue. One question: What would happen if the time resolution wasn't 1 hour but, let's say, 3 hours? This would still work right?

Yes, it works at any time sampling. I can add a unit test for that to this PR if you like.

That said, there does seem to be a slight difference in the values returned from CDS between 1-hour sampling and all other samplings (I tried 2h, 3h, 4h, and 6h) - see my comment here: https://github.com/PyPSA/atlite/issues/256#issuecomment-1271384837

zoltanmaric commented 1 year ago

@FabianHofmann I added a test for 3h-sampling in https://github.com/zoltanmaric/atlite/commit/17a08475b4c3d97d3a380ea8a2d4b9fb3e6c3b2c. Let me know if you want to keep it or if I should remove it.

FabianHofmann commented 1 year ago

@FabianHofmann I added a test for 3h-sampling in zoltanmaric@17a0847. Let me know if you want to keep it or if I should remove it.

Let's keep the test. As far as I see, only a release note is missing. Would you be so kind? Then we can merge it.

zoltanmaric commented 1 year ago

As far as I see, only a release note is missing. Would you be so kind? Then we can merge it.

I wasn't sure about the format - I added a new version heading (0.2.10) and added a note there (see here). Is that right?

FabianHofmann commented 1 year ago

Thanks @zoltanmaric

PyPSA / atlite