Skip to content
This repository was archived by the owner on Sep 11, 2023. It is now read-only.

Update Satellite Zarr reader for new format #313

Closed
Tracked by #393
jacobbieker opened this issue Nov 1, 2021 · 7 comments · Fixed by #339
Closed
Tracked by #393

Update Satellite Zarr reader for new format #313

jacobbieker opened this issue Nov 1, 2021 · 7 comments · Fixed by #339
Assignees
Labels
data New data source or feature; or modification of existing data source enhancement New feature or request

Comments

@jacobbieker
Copy link
Member

Detailed Description

Satellite data is now stored in the zarr in unprojected format, so the original geo projection. This does make it slightly more difficult to save the OSGB coordinates in the zarr, as each pixel has a unique OSGB x and y coordinate. Currently, they are stored in the metadata field, under 'osgb_x' and 'osgb_y'. There might be a better way to store it, which we could also do, but for now, we probably need to read in the metadata of one example each for the HRV channel and all the other channels, to get the mapping from image coordinates to OSGB coordinates.

Context

Possible Implementation

@jacobbieker jacobbieker added enhancement New feature or request data New data source or feature; or modification of existing data source labels Nov 1, 2021
@jacobbieker jacobbieker moved this to Todo in Nowcasting Nov 1, 2021
@jacobbieker jacobbieker self-assigned this Nov 2, 2021
@jacobbieker
Copy link
Member Author

Will start on this once the big redesign #307 is merged!

@jacobbieker
Copy link
Member Author

@peterdudfield @JackKelly For this, what do you guys think about just adding a second Sattelite source to the Batch? Then we give one the HRV Zarr and one the normal Zarr, and nothing else needs changing?

@peterdudfield
Copy link
Contributor

Whats the advantage of this?

There is something nice, about just having it in one, and perhaps the satellite xr.Dataset loading both of them? Perhaps they are singificant different to suggest they should be in a different 'DataSource'

@jacobbieker
Copy link
Member Author

jacobbieker commented Nov 5, 2021

The advantage is minimal code changes, the HRV DataArray we can't add to the non-HRV DataArray because of the different resolutions. And to cover the same area, the HRV cutout has to be 3x the size of the non-HRV image. Other than that, they would be the same, but the different size/shape of an HRV example vs non-HRV example suggests to me it might make sense to keep them as separate entries?

Also, unlike the separate Optical Flow Data source, it wouldn't be loading the same data multiple times, if they are different data sources. Keeping them as the same one would then involve a decent amount of checks for if one, or both of the non-hrv and hrv data was loaded. So makes the code a bit messier.

@JackKelly
Copy link
Member

Yeah, I'd go with whatever is simplest to implement :)

@jacobbieker
Copy link
Member Author

Cool, I'll do the HRV channel as a separate data source then!

@JackKelly
Copy link
Member

Sounds good

Although, that said, I do agree with @peterdudfield that it feels kinda weird having two SatelliteDataSources... 🙂 but let's give it a go and see how it feels!

@JackKelly JackKelly moved this from Todo to Reviewer Approved in Nowcasting Nov 17, 2021
Repository owner moved this from Reviewer Approved to Done in Nowcasting Nov 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
data New data source or feature; or modification of existing data source enhancement New feature or request
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants