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

Smarter merging of band listings in federated collections #147

Open
soxofaan opened this issue Jun 27, 2024 · 2 comments
Open

Smarter merging of band listings in federated collections #147

soxofaan opened this issue Jun 27, 2024 · 2 comments

Comments

@soxofaan
Copy link
Member

Triggered from https://discuss.eodc.eu/t/valueerror-invalid-band-name-index-scl-valid-names-b01-b02-b03-b04-b05/765:
User wants to use SCL band from SENTINEL2_L2A collection (which used to work), but that fails now because SENTINEL2_L2A is federated/merged collection from Terrascope and EODC and EODC version does not provide a SCL band, so aggregator does not expose that band (even though it could be provided by Terrascope).

In the current implementation the aggregator merges collections by taking the intersection (common prefix actually) of the band listings provided by upstream backends, because that is the easiest to do in a reliable and predictable manner.
Note that things like reduce_dimension along band dimension often heavily rely on band index, so things will go wrong unnoticeably when the aggregator messes with the band ordering under the hood.

However, it is clearly not ideal that band listings of federated collections are highly dependent on consistency and detailed harmonization across all upstream backends.

Instead the aggregator could be smarter about this:

  • provide union of bands instead of intersection
  • route processing requests based on requested bands in process graph
  • properly act on processing steps that depend on band order that is unreliable: e.g. throw an error or automatically convert processing to more reliable approach (e.g. use band labels instead of index in array_element)
@jdries
Copy link
Contributor

jdries commented Jun 27, 2024

@soxofaan if this is a regression, can you configure whitelisting appropriately so that users can continue working till it's fixed? Seems like we should do this rather fast?

@soxofaan
Copy link
Member Author

Yes, I'll update the configuration so that only Terrascope is considered for SENTINEL2_L2A. But this is of course more a quickfix than a structural solution

soxofaan added a commit that referenced this issue Jun 27, 2024
soxofaan added a commit that referenced this issue Aug 30, 2024
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