-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improved metadata #240
Improved metadata #240
Conversation
- Add starter code for clay v1 model - Add `model_clay_v1.py` that handles images of different size & spectral bands - Add `metadata.yaml` that contains meta information of satellite data, like wavelengths & gsd - Add cls tokens to ClayMAE
…nsforms with v2.transforms
PyTorch Image Models!
- First working version of sampler --------- Co-authored-by: Daniel Wiesmann <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @yellowcap, I'm just cross-checking the wavelengths for now which look mostly ok. I do have a comment/suggestion about how we might want to consistently name the Near-infrared band(s) across Sentinel-2 and Landsat (and potentially NAIP).
band_order: | ||
- vv | ||
- vh | ||
gsd: 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that there is a subtle difference between Ground Sampling Distance (GSD) and pixel spacing (see e.g. https://mapscaping.com/understanding-ground-sampling-distance-gsd). The more technically correct term here would be pixel spacing, as the Sentinel-1 image is delivered as images with 10m pixels, but the actual ground sampling distance is more like 20m (xref https://sentinel.esa.int/web/sentinel/technical-guides/sentinel-1-sar/products-algorithms/level-1-algorithms/ground-range-detected/iw).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very good point. Maybe this could be renamed to pixel spacing. I think this is what the model need to know the most here, what size a single pixel represents. The GSD would be good additional information, but it would have to be specified per band for multiple systems. For instance, for Sentinel-2 we resample everything to a pixel spacing of 10m, although some bands have 20m GSD. So lets keep it as is for now and update naming later?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, we can keep the naming for now (otherwise the model code needs to be updated too).
- nir08 | ||
- swir16 | ||
- swir22 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the near-infrared band for Landsat called nir08? Band 8 on Landsat 8/9 is the panchromatic band. Is this to match with Sentinel-2? Maybe we could follow the HLS naming scheme at https://lpdaac.usgs.gov/data/get-started-data/collection-overview/missions/harmonized-landsat-sentinel-2-hls-overview/#hls-spectral-bands if we want to keep the same band name. I.e. use NIR Broad, NIR Narrow, SWIR 1, SWIR 2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The naming comes as-is from the underlying STAC catalog. We have this baked into the code in 2-3 places now. So let's not change it right now and make this more consistent in a future iteration.
Example:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see, then let's keep this as is for now then.
This needs another sanity check, and the actual band stats are in computation right now. Should update tomorrow morning
for more information, see https://pre-commit.ci
Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
Co-authored-by: Wei Ji <[email protected]>
bed94c4
to
4c0c846
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-approving to move this along (you'll need to fix the merge conflicts first though). Still think we need to move away from mean/std normalization (#94), but could do it for model v2 I guess 🙂
This has been merged in #253 |
This needs another sanity check, and the actual band stats are in computation right now. Should update tomorrow morning with the stats from the calculations on batch.