Closed peterdudfield closed 1 month ago
Im unsure where to just make GSP optional, all make a dummy zarr file with fake gsp results.
The dummy file feels like we shouldnt need it, but then adding sun position gets hard without gsp.
@dfulu what do you reckon? Maybe the dummy zarr might be easier
I think we should refactor add_sun_position to not require the GSP data. That is planned in #20
I think this should be easier to do once #27 is merged
Yea thanks, yea it should be easier. Unfortunately it still needs the GSP config, which i was trying to get rid of in live. Mokcing the GSP zarr would still be an option.
Other option is to have a training=true
, but I like the idea of not using this as it runs the risk of the training and inference pipeline being very differnet
One other option could be to have a new sun configration, that we use for making sun coordiantes. We could have some deafult values here that would be basically the same forecast minutes as gsp
Yeh probably having a separate entry in the config would be best. Maybe we should go ahead and bring the datapipes config in here now?
Yeh probably having a separate entry in the config would be best. Maybe we should go ahead and bring the datapipes config in here now?
yea why not, can you copy over tests e.t.c too?
Again, it would be great if it was backwards compatiable. I.e if we add sun config, lets add default values to it, so even if an old config doesnt have sun, there will be values
The more issues that come up, the more I think we shouldn't aim for backwards compatibility. In the old version we add the sun position to match the input modality. i.e. to match the datetimes for GSP, or PV, or even wind. So which of these modalities would we make the config default to?
As far as I'm aware Windnet does use the solar coords with the wind modality, but PVNet UK regional uses the GSP modality, and I think PVNet Rajasthan uses the PV modality
And also it would be good to be able to change the config to match the parameter naming of #22.
As far as I'm aware Windnet does use the solar coords with the wind modality
It doesn't anymore, we ended up stripping that out when I was adding explicit time features, it uses this instead.
It is very tempting to me to not aim for backward compatibility, yes. I think otherwise we will end up keeping things we don't like but are "fine"/adding props that will need to be stripped out once the migration is complete but will be inevitably forgotten/something else will depend on them by then
But also re: having sun position default to something/completely divorced from GSPs, there're already default settings in configs (for history and forecast duration, might even be for time resolution as well but I doubt it; but in any case, surely we can settle on a default time res)? Why can't we use those?
Or maybe even get min/max of intervals and time resolutions of sources? This way we can get exact coverage for the time window we're using at the highest granularity present. I don't know, adding separate configs for it seems odd to me; it kind of makes it its own data source, when in reality it's just a function of preexisting coordinates
Could we create an issue called 'moving configuration from ocf_datapipes` and then chat about it there?
Detailed Description
Currently GSP dataset is needed, but for live inference we infact dont need this. We will need to use a certain t0 in live
Context
Possible Implementation