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

CARD4L SAR Extension (NRB + POL) #922

Closed
wants to merge 40 commits into from
Closed

CARD4L SAR Extension (NRB + POL) #922

wants to merge 40 commits into from

Conversation

m-mohr
Copy link
Collaborator

@m-mohr m-mohr commented Nov 23, 2020

The STAC CARD4L SAR Extension has been migrated to a separate repository: https://github.com/stac-extensions/card4l/tree/main/sar

@m-mohr m-mohr mentioned this pull request Nov 24, 2020
4 tasks
@fangfy
Copy link

fangfy commented Nov 26, 2020

  • In the current xml, IncAngleNearRange and IncAngleFarRange are for the source data, which means this doesn't necessarily correspond to actual incidence angle range for this output. If the view:incidence_angle is intended to be for the sensor, then it's appropriate to use the middle point angle.
  • The SAR extension has a few product type defined, so it should be RTC in that list (CARD4L requires RTC). However, the Sentinel-1 product has backscatter provided as power/intensity, not amplitude. Therefore, an additional field is needed in the sar extension.
  • There are a few pieces of important information related to processing that's not yet in STAC, e.g. filtering (whether a speckle filter has been applied to the data), thermal noise removal (whether it has been applied), not sure whether they should be in "about".
  • where does processing time and software + version go in STAC?

Copy link
Contributor

@cholmes cholmes left a comment

Choose a reason for hiding this comment

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

Looks great. One minor typo fix that I tried to put as a commitable suggestion. Psyched to see this added to STAC.

extensions/card4l-sar-nrb/README.md Outdated Show resolved Hide resolved
m-mohr and others added 3 commits November 30, 2020 18:16
@m-mohr
Copy link
Collaborator Author

m-mohr commented Dec 10, 2020

@fangfy Thank you for your comments, very much appreciated! Sorry for the delay, I was waiting for #907 to be merged.

  • In the current xml, IncAngleNearRange and IncAngleFarRange are for the source data, which means this doesn't necessarily correspond to actual incidence angle range for this output. If the view:incidence_angle is intended to be for the sensor, then it's appropriate to use the middle point angle.

I understand the spec the way that view:incidence_angle indeed is intended to be for the sensor.

  • The SAR extension has a few product type defined, so it should be RTC in that list (CARD4L requires RTC). However, the Sentinel-1 product has backscatter provided as power/intensity, not amplitude. Therefore, an additional field is needed in the sar extension.

@matthewhanson What do you think? You proposed RTC, but what would be the preferred way out? Should we add more product types for SAR or go another route?

  • There are a few pieces of important information related to processing that's not yet in STAC, e.g. filtering (whether a speckle filter has been applied to the data), thermal noise removal (whether it has been applied), not sure whether they should be in "about".

We are likely not capturing all details in STAC, but they are available in the additional XML file that is required anyway, right?

  • where does processing time and software + version go in STAC?

We've recently introduced the processing extension with PR #907. There are now fields for things like software + version and I'll add that now to the proposal.

@fangfy
Copy link

fangfy commented Dec 11, 2020

  • There are a few pieces of important information related to processing that's not yet in STAC, e.g. filtering (whether a speckle filter has been applied to the data), thermal noise removal (whether it has been applied), not sure whether they should be in "about".

We are likely not capturing all details in STAC, but they are available in the additional XML file that is required anyway, right?

CARD4L doesn't require XML, if all required metadata can be provided in STAC, then only one metadata file is needed.

@m-mohr m-mohr changed the title CARD4L SAR Normalized Radar Backscatter (NRB) Extension CARD4L SAR Extension (NRB + POL) Dec 12, 2020
@m-mohr m-mohr force-pushed the card4l-sar-nrb branch 2 times, most recently from efc58b0 to da7e36f Compare December 12, 2020 13:14
@m-mohr
Copy link
Collaborator Author

m-mohr commented Dec 14, 2020

I've finished writing up the document that captures (I hope) all requirements from CARD4L SAR NRB and POL in a single STAC extensions. It got pretty heavy and was quite a bit of effort, but I've now also included a "pure" STAC mode, if you don't want the XML derived from the CARD4L metadata spec. The question is how to proceed with this? Is there a way to get that through some CEOS review or so? For example there is an open question re 1.7.3 in the document... In the best case we could finalize it with CEOS and actually make it somewhat official or so. Read it here: https://github.com/radiantearth/stac-spec/blob/card4l-sar-nrb/extensions/card4l-sar/README.md
@fangfy @alexgleith

@m-mohr m-mohr force-pushed the card4l-sar-nrb branch 2 times, most recently from 791d74b to 67d1132 Compare December 15, 2020 13:21
@fangfy
Copy link

fangfy commented Feb 1, 2021

Thank you, @fangfy. I highly appreciate that you took the time and have added your feedback here. It's just in time for us to implement it. I've applied all suggested changes and updated the document accordingly, but some additional comments below:

  1. Re 1.7.3: ProductColumnSpacing, ProductRowSpacing are the actual product sampling. They should be in meter or degree (in ground geometry as required by CARD4L, therefore different to the sar:pixel_spacing_azimuth and sar:pixel_spacing_range which are in radar geometry).

In STAC sar:pixel_spacing_azimuth and sar:pixel_spacing_range are always in meters, so it sounds like they could really be used for Src and Prod. I've changed the document accordingly.

Problem is the _azimuth and _range part, which are relative to sensor not ground, so they don't apply to card4L product.

  1. the metadata assets (2.3 -2.6) should have relevant unit defined.

I've added a card4l:unit flag that can be set, but we could maybe remove it if it's always the same unit?

The units are different for the different images, hence it's important.

  1. for the backscatter image assets (or as overall metadata), BackscatterMeasurement (should be gamma0 for CARD4L, but for other radar product, this can be sigma0, beta0 etc), and BackscatterConvention (amplitude or power, this is important!) should be defined. These two (and probably a few others) would make most sense to be included in the sar extension.

I couldn't find these metadata entries in the official CARD4L SAR NRB metadata specification. Have I missed them? If they are missing there, why are they missing there if this information is important? Also, which part of the specification requires them? It's a bit strange to require them but not list the requirement number.

They are listed as parameters in the backscatter measurements (3.1). They do apply directly to the backscatter images/assets.

I hope these help to clarify. Thanks heaps for implementing these!

@m-mohr m-mohr mentioned this pull request Feb 2, 2021
4 tasks
@m-mohr
Copy link
Collaborator Author

m-mohr commented Feb 2, 2021

Thank you again, @fangfy!

Problem is the _azimuth and _range part, which are relative to sensor not ground, so they don't apply to card4L product.

In #962 I'm now proposing some additional fields to be added to the SAR extension. I hope I understood things correctly with my limited SAR knowledge. Thus there are separate fields for row and column. I'd appreciate if you could have a look at PR #962.

The units are different for the different images, hence it's important.

I was not giving the idea clear enough, I think. I meant if it's a fixed unit for the individual assets, one may not give them, but I'm fine to just leave them in and make it explicit.

They are listed as parameters in the backscatter measurements (3.1). They do apply directly to the backscatter images/assets.

Thank you, I found them now and also looked at POL again to align better. I have also tried to add those to the SAR extension directly, see #962.

@m-mohr m-mohr mentioned this pull request Feb 2, 2021
4 tasks
@m-mohr m-mohr marked this pull request as ready for review February 3, 2021 15:50
@m-mohr
Copy link
Collaborator Author

m-mohr commented Feb 23, 2021

The STAC CARD4L SAR Extension has been migrated to a separate repository: https://github.com/stac-extensions/card4l/tree/main/sar

@m-mohr m-mohr closed this Feb 23, 2021
@m-mohr m-mohr deleted the card4l-sar-nrb branch February 23, 2021 17:24
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.

3 participants