Always shift solar influx by 30 minutes #257
Conversation
There was a problem hiding this comment.
@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: #256 (comment) |
| SARAH_DIR = os.getenv("SARAH_DIR", "/home/vres/climate-data/sarah_v2") | ||
| GEBCO_PATH = os.getenv("GEBCO_PATH", "/home/vres/climate-data/GEBCO_2014_2D.nc") | ||
|
|
||
|
|
There was a problem hiding this comment.
This section was just moved to the top of the file, because I added a reference to TIME in one of the methods above.
|
@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. |
zoltanmaric
force-pushed
the
time-shift-30min
branch
from
4de756a to
17a0847
Compare
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
force-pushed
the
time-shift-30min
branch
2 times, most recently
from
da9eb14 to
9faef23
Compare
I wasn't sure about the format - I added a new version heading ( |
zoltanmaric
force-pushed
the
time-shift-30min
branch
from
9faef23 to
f2c6e3b
Compare
RELEASE_NOTES.rst
Outdated
| Version 0.2.10 | ||
| ================= |
There was a problem hiding this comment.
| Version 0.2.10 | |
| ================= | |
| Upcoming Release | |
| ================ |
There was a problem hiding this comment.
(otherwise there will be a warning during the compilation)
There was a problem hiding this comment.
OK, I also noticed in the meantime that the latest release is 0.2.8. Should I then add my note to version 0.2.9 instead?
There was a problem hiding this comment.
No, there was just the GH release missing (which is sort of secondary). But perhaps you can name the heading Upcoming Release?
zoltanmaric
force-pushed
the
time-shift-30min
branch
from
f2c6e3b to
28615a3
Compare
|
Thanks @zoltanmaric |
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 #256 (comment) for reasoning.
Motivation and Context
Closes #256
How Has This Been Tested?
Type of change
Checklist
pytestinside the repository and no unexpected problems came up.doc/.environment.yamlfile.doc/release_notes.rst.pre-commit run --allto lint/format/check my contribution