Bug fix on GCM clearsky ratio #249
Conversation
…, need to rescale when loading GCM rsds
|
|
||
| cs_ghi = cs_ghi[..., : len(self.rasterizer.time_index)] | ||
| cs_ghi = da.concatenate(multi_year, axis=-1) | ||
| cs_ghi = cs_ghi[..., :len(self.rasterizer.time_index)] |
There was a problem hiding this comment.
Maybe this would be cleaner if we just matched the doy in cs_ghi.time with the doy in self.rasterizer.time_index for each element?
There was a problem hiding this comment.
Very good call. Check this out. Will want to test on large timeseries once kestrel is back up but i think this should work!
2d33c8b#diff-d97151971c312b63001f37eafc4f5fac2020d7b0a7421d7b3e4adfa1bf7d1cebR218-R222
There was a problem hiding this comment.
Yeah that looks great! Little nitpick 'time' -> Dimension.TIME. I don't think pd.to_datetime() is needed here either, but doesn't matter
There was a problem hiding this comment.
hmmm maybe im misunderstanding but i dont see the benefit of storing the index up at line 207 vs. pulling it at 220 and converting to month-day. I kind of like having everything contained in these few lines that reindex the ti. I should def be using Dimension.TIME though. will push that.
There was a problem hiding this comment.
Yeah, disregard that initial message. At first glance I thought there was some extra unneeded conversion happening. Edited to clarify.
There was a problem hiding this comment.
Gotcha! I think I saw the time index is stored as a numpy array so needed the to datetime. doesnt hurt anyway.
… lots of variability with such small test sample
grantbuster
force-pushed
the
gb/csratio
branch
from
13b11dc to
2d33c8b
Compare
…reindex function with month-day strings
grantbuster
force-pushed
the
gb/csratio
branch
from
2d33c8b to
098f956
Compare
|
Just verified this functionality on kestrel with a 10 year timeseries, everything works as expected, merging. |
Bug fix on GCM clearsky ratio
@bnb32, possibly a bit premature as i haven't tested thoroughly yet, but since kestrel is down and i know you're bored i thought i'd submit this for review now.
Main fix is that
da.repeatdoes not do what we think it does for multi year solar timeseries (see lines referenced below). We need to stack multiple years of the NSRDB clearsky ghi but da.repeat does [1, 2, 3] -> [1, 1, 2, 2, 3, 3] so you get one giant annual cycle which results in a lot of ratio=1 values at the edges. Leap days also matter... if we do nyears*366 for nyears>10 we end up with a growing shift in the later years. This matters a lot for the 40-year record we analyze for bias correction.main...gb/csratio#diff-d97151971c312b63001f37eafc4f5fac2020d7b0a7421d7b3e4adfa1bf7d1cebR223-R229