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

improvement spectral indices functions #501

Closed
mbuchhorn opened this issue Nov 10, 2023 · 3 comments
Closed

improvement spectral indices functions #501

mbuchhorn opened this issue Nov 10, 2023 · 3 comments
Assignees

Comments

@mbuchhorn
Copy link

mbuchhorn commented Nov 10, 2023

The spectral indices functions in openEO show some important shortcoming to be production ready. There have to be several fixed implemented:

  1. the awesome package provides next to the [spectral-indices-dict.json] also a [bands.json] which should be used to assign the datacube band names to the unified band names used by the awesome package --> instead of the hardcoded translation currently used in the source code --> still sensor definition via the catalog_ID is possible, but then the band translations should be pulled from the json files for the sensor
    ==> also do not forget to implement issue Extend applicability of "awesome spectral indices" wrapper beyond pure load_collection data #485

moreover issues that the Sentinel2 bands are not correctly defines in the code is then also obsolete.... e.g. B8A is named N2 in the awesome package and NOT RE4 as in the openEO
see BAND_MAPPING_SENTINEL2 = {
"B1": "A",
"B2": "B",
"B3": "G",
"B4": "R",
"B5": "RE1",
"B6": "RE2",
"B7": "RE3",
"B8": "N",
"B8A": "RE4",
"B9": "WV",
"B11": "S1",
"B12": "S2",
}

  1. some of the VI's defines in the awsome package need CONSTANTS --> these have to be added to the approach and should be loaded from [constants.json] !!!
    Note: the CONSTANT lambdaG, lambddaN and lambdaR have to be filled by the specification of the sensor (center wavelength of the band of the used satellite)

  2. the usage of the extra-indices-dict.json in the load_indices() function must be checked..... currently some indices are overwritten which have the same name but in the awesome package a complete different definition. That is a nogo! If additional VI wants to be added then make sure they have a clear new name and not overlapping with the awesome package definitions.

@soxofaan
Copy link
Member

Already did a first couple of improvements:

  • the "constants.json" file is now included, and basic support for constants is in place: e.g. EVI works now (required a couple of constants defined in "constants.json"). Note working with sensor-dependent "constants" is not supported yet
  • removed "NDGI", "NDMI" and "S2WI" from "extra-indices-dict.json" because these were indeed shadowing the official definitions from Awesome Spectral Indices. I think we should get rid of this whole "extra-indices-dict.json" file and need a different mechanism to work with custom/alternative index definitions.

@soxofaan
Copy link
Member

Now also added a new argument platform to append_index, compute_index, ... to explicitly specify the satellite platform if the cube metadata has no id (or it's unhandled). e.g.

cube = compute_index(cube, index="NDVI", platform="Sentinel2")

soxofaan added a commit that referenced this issue Nov 15, 2023
soxofaan added a commit that referenced this issue Nov 16, 2023
Use mapping data from Awesome Spectral Indices
Allow custom mappings
soxofaan added a commit that referenced this issue Nov 16, 2023
Use mapping data from Awesome Spectral Indices
Allow custom mappings
soxofaan added a commit that referenced this issue Nov 16, 2023
mapping from variable to band is less error prone than the other way around
soxofaan added a commit that referenced this issue Nov 16, 2023
@soxofaan
Copy link
Member

soxofaan commented Nov 16, 2023

As mentioned in #485 I now also added option to manually specify the band mapping. See documentation at https://open-eo.github.io/openeo-python-client/cookbook/spectral_indices.html#band-mapping

(these features are not released yet, will be for next release 0.26.0 FYI)

I think this resolves the most important issues raised here:

instead of the hardcoded translation currently used in the source code .. the band translations should be pulled from the json files for the sensor

done

also do not forget to implement issue #485

done

Sentinel2 bands are not correctly defines in the code is then also obsolete.... e.g. B8A is named N2 in the awesome package and NOT RE4 as in the openEO

done

some of the VI's defines in the awsome package need CONSTANTS --> these have to be added to the approach and should be loaded from [constants.json]

done

Note: the CONSTANT lambdaG, lambddaN and lambdaR have to be filled by the specification of the sensor (center wavelength of the band of the used satellite)

Note done yet, I will make a separate ticket for that -> #507

the usage of the extra-indices-dict.json in the load_indices() function must be checked..... currently some indices are overwritten which have the same name but in the awesome package a complete different definition

done for indices that were being overwritten. Completely getting rid of extra-indices-dict.json is for #506

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

No branches or pull requests

2 participants