Can we simplify the code by always keeping the data in one data type (e.g. xr.DataArray) per modality?

[JackKelly]

In the past, nowcasting_dataset was designed to feed data on-the-fly into a PyTorch model. Which meant that, as the data flowed through nowcasting_dataset, the data would change type: For example, satellite data would start as an xr.DataArray, then get turned into a numpy array (because PyTorch doesn't know what to do with an xr.DataArray), and then get turned into a torch.Tensor.

But, I think we can safely say now that nowcasting_dataset is just for pre-preparing batches (not for loading data on-the-fly). As such, we can probably simplify the code by keeping data in a single container type per modality. For example, satellite data could always live in an xr.DataArray for its entire life while flowing through nowcasting_dataset.

Sorry, I really should've thought of this earlier! But, yeah, I think this could simplify the code quite a lot.

I haven't fully thought through the implications of this, but some changes might be:

• In the Pydantic models, each field can be just a single type (instead of a Union of types). So, for example, instead of sat_data: Array = Field(... we can just do sat_data: xr.DataArray = Field(...
• We can get rid of the to_numpy function.
• For all the modalities which use xarray of pandas data types, we can use dimension names instead of indexes. e.g. seq_length = len(sat_data[-4]) becomes seq_length = len(sat_data.time)
• Saving and load data to/from disk becomes super-simple.
• We'd no longer need the to_xr_dataset and from_xr_dataset methods (which are quite fiddly)

[JackKelly]

And then, I wonder if DataSourceOutput could become a mixture of a Pydantic model (so we get the typing and the validation) and an xr.Dataset (so we get all the goodies that come with xr.Datasets, like plotting and disk IO?) To be specific, we could do:

class DataSourceOutput(pydantic.BaseModel, xr.Dataset):

I haven't tested this, so I have no idea if it'll actually work!

[JackKelly]

Before we can fully implement this, #86 probably needs to be implemented

[JackKelly]

I've asked for advice on twitter :)

[gsvitak]

@JackKelly Although I do not full understand the use case, I would suggest to check out a Mixin in python. https://www.thedigitalcatonline.com/blog/2020/03/27/mixin-classes-in-python/

[peterdudfield]

Looks like you would still need a to_tensor() function that moves the xr datasets to tensors

https://github.com/openclimatefix/nowcasting_dataset/blob/peter/explore/notebooks/2021-10/2021-10-11/xr_tensor.py

Unless there is a better way?

[JackKelly]

Yes, I agree, we'd definitely still need a to_tensor() function to go from xr.Dataset to torch.Tensor :)

I've been updating issue #25 with some notes about that; and your approach looks good to me, @peterdudfield !

[peterdudfield]

Some more playing here

https://github.com/openclimatefix/nowcasting_dataset/blob/peter/explore/notebooks/2021-10/2021-10-12/Pydantic%20models.ipynb

[JackKelly]

Looks good!

Another simplification that we might be able to make is to only represent batches (#212). In other words, we could drop the idea of representing individual "examples" 🙂. I haven't fully thought this through but I think that it was only necessary to represent individual examples back when we wanted nowcasting_dataset to combine multiple modalities into a single batch during pre-processing. But, once #202 is implemented, I think we may be able to move to a world where each DataSource only loads entire batches for pre-processing 🙂

So, in your ipython notebook, we might be able to drop SatelliteBatch becuase Satellite will always represents a whole batch of satellite data :)

Does that sound OK? Or have I gotten myself confused?!

[peterdudfield]

That does make sense,

yea I would expect each 'datasource' to make a BatchObject and save it to a file. So in that case 'Satellite' could be ignored.

I agree Example wont really be needed, in this ml pipeline.
But it might be useful to have this for visualisation. I kinda of like the idea of showing the whole example and try understanding to see the behaviour

[peterdudfield]

just to see it in a drawing