Skip to content
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

Added rededge 1-3 as common names #19 #22

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Added rededge 1-3 as common names #19 #22

wants to merge 2 commits into from

Conversation

m-mohr
Copy link
Contributor

@m-mohr m-mohr commented Jan 9, 2023

Implements #19

Copy link
Member

@emmanuelmathot emmanuelmathot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a discrepancy regarding existing splitted CBN (e.g. nir08, nir09, swir16, swir22)
Either we keep splitting the common band with names based on their subranges or we use agnostic division number as in https://github.com/awesome-spectral-indices/awesome-spectral-indices#expressions.
The former is not ideal since all instruments do not share the same wavelengths division but has the advantage of a semantic information in the band name.
The latter is more flexible but we loose an information and in addition this would be a breaking change for future versions.

Personally, I'd rather keep the subrange information in the CBN.

@m-mohr
Copy link
Contributor Author

m-mohr commented Jan 9, 2023

So my use case is exactly "Awesome Spectral Indices". I'd like to apply a spectral index from the list to data and just get the correct assets (bands) from the metadata based on the common name (or wavelength, if no common name is present). This only works right now if the formula doesn't include a rededge1,2,3 band. The overlapping wavelengths (and CBNs) are an issue though.

@emmanuelmathot
Copy link
Member

emmanuelmathot commented Jan 11, 2023

I understand you requirement, I have the same since 2 years and I even needed new bands to be added in the CBN list. I pushed a PR at that time. Beyond the fact that the rule is that any new CBN must be shared by 3 or more other satellites, my comment here is more about the consistency of the nomenclature we adopt. Either a simple division number or an average wavelength number suffix (e.g. rededge70, rededge75).

@philvarner
Copy link

S2 red bands are:

  • B05 | Vegetation red edge, 704.1 nm (S2A), 703.8 nm (S2B) | 20m
  • B06 | Vegetation red edge, 740.5 nm (S2A), 739.1 nm (S2B) | 20m
  • B07 | Vegetation red edge, 782.8 nm (S2A), 779.7 nm (S2B) | 20m

@m-mohr
Copy link
Contributor Author

m-mohr commented Jan 30, 2023

Meeting notes: Needs a broader discussion, but rededge1-3 are not consistent with e.g. nir08 etc so we may want to change then to e.g. regedge70, rededge75 etc (as proposed by Emmanuel above). For this it would also be useful to know how other satellites do it and which bands they have. So I'll look that up and propose a name. Also we should check with ASI how they differentiate.

@davemlz
Copy link

davemlz commented Feb 3, 2023

Hey! Sorry to just jump into the discussion. @m-mohr opened and Issue in ASI (awesome-spectral-indices/awesome-spectral-indices#30) (I just renamed it), and it would be cool to have the mapping from the ASI standard to this standard. So, just to give you an overview, the bands and their standard in ASI were selected according to bands required by specific spectral indices (a band in a specific index shouldn't be ambiguous). E.g., in the case of Sentinel-2, there are indices that require the three red edge bands (see IRECI = (RE3 - R) / (RE1 / RE2)) -> This index couldn't be computed if the red edge bands were just a common unique band.

@m-mohr
Copy link
Contributor Author

m-mohr commented Feb 7, 2023

Interesting resource: https://eoreader.readthedocs.io/en/v0.19.1/optical_band_mapping.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add rededge1,2,3 to common names
4 participants