Closed RohdeBSH closed 1 month ago
From my point of view this is ready for review. Perhaps @AnnaWall01 and/or P-O could take a look first in order to catch any parts that might need further work.
@AnnaWall01 This is redy for your approval (pending your review). After your approval this can be merged.
Reference email from Anna pm 4 APR 2024, Notice period for Pull Request #73. Some thoughts from our NIWC team.
We have some concerns that what is presented here deviates from the ISO 19123 / OGC 07-011.
I don’t have the ISO, but from OGC 07-011:
The grid origin is clearly defined as the 0,0 position for the entire grid, by fundamental definition. There are other pieces to this puzzle that ripple out from this, but this is a foundational definition. S-100 Edition 5.1 section 8-5.2.8 Grid cell structure, introduces “offsets” that further complicate and deviate from the source ISO specs.
In S-100 this is captured in Table 10c-10 – Attributes of feature container groups, "dataOffsetCode". But this is only in the ENCODING and we can’t find it in the S-100 general feature model. The “dataOffsetCode” appears to only apply to the data values (grid points), not to the origin, but it is not entirely clear. Anyway, in total, this makes the encoding specific to HDF5 products, we think.
As such, “dataOffsetCode” is a bad idea because it deviates from the ISO spec, but nonetheless it is in S-100 for HDF5 encoding only and you can technically use it. I personally find it to further confuse the definitions of everything grid related, whereas the ISO spec clearly establishes the relationships.
In summary, from your email diagram below, the “X”, (grid origin), should be at X0, Y0 if you want to be maximally conformant to the ISO specs.
The comment above by @mikan66 is related to issue #83. This issue was created based on earlier comment on this PR.
To change the product specification from being Node- to Cell area-based is possible based on functionality/metadata/attributes currently available in S-102 PS and S-100 ed. 5.0.0. Our suggestion (in dialog with S-100) is changing the encoding in section 10.2.4. Root BathymetryCoverage more specifically the attributes of the BathymetryCoverage feature container group.