Review - 3.0.0 - BSH findings

[RohdeBSH]

Based on artefact from 03.07.2024 19:37

General

• Some section seems not to be formatted as justified text. Is that indented?

  2.2. References

• Reference to S-100 is not displayed (HTML)
  • Reference in S-102 adoc looks good
  • Issue is may located in relation-iho or relation-data-iho
  • relation-data-iho looks also good
  • ask metanorma guys

    2.3.3. Abbreviated terms

• GFM is not contained in the text of the document
• SA is not contained in the text of the document

  4. Data Product Identification

• Classification -> 8. Protected; or -> the “or” can be omitted. The senctence above states “[…] one of the following:”, so there is no need of the “or”.

  5.2.1.1.1.8. dimension

• States that the value 2 is fixed and does not need to be encoded.
  • But in 11.2.4 Table 7 the dimension is mandatory to encode
  • S-100 5.2.0 Table 10c-10 also states dimension as mandatory to encode
  • The statement of this clause does not match the encoding rules

    5.2.1.1.1.11. sequencingRule

• There is no class with the name _“CVSequenceRule” in S-100 5.2.0
  • The class _“CVSequenceRule” is descripted in the S-100 2.0.0 UML model only.
  • sequenceRule is a complex attribute containing the attribute scanDirection and type
  • Only the value for sequenceRule.type can be “linear”. The clause is somehow misleading.
    • What is meant here is _“CVSequenceType” see S-100 5.2.0 cl. 8-7.7.2 _“CVSequenceType” contains the value “linear”

      5.2.1.1.2.2. values

• Reading the last sentence of the clause sounds strange to me. But I’m a non-native speaker.
  • […] _There are two attributes in the bathymetry value record, depth and optional uncertainty in the S102BathymetryValues class.

[hasel001]

Addressed dimension, sequencingRule, and values here.

[hasel001]

Addressed unused abbreviations here. Bad S-100 5.2.0 link is known, and I will ensure I address it upon the next release of metanorma.

[hasel001]

Addressed extraneous "or" here. Thank you for the review!

[RohdeBSH]

I wasn't finished yet. The weekend just came quicker than expected and I had to document it. ;)

[hasel001]

@RohdeBSH Sorry for the premature closure.

[RohdeBSH]

Based on artefact from 07.07.2024 18:39:13 GMT

2. Overview [PDF]

• placement is disordered again
  • Correct placement Contents section
  • Incorrect placement within the document (page numbers)

    5.7 Geometry

• Reference is not correct defined

  6.4 Temporal reference system

• Attributes with S-100 dateTime-Format (S-100 5.2.0 Part 1 clause 4.5.2 Table 1-2 & S-100 5.2.0 Part 10c clause 7 Table 10c-1) are forced to be in UTC (because of the Z at the end of the String).
  • Does this also apply to the attribute issueTime, so to attributes in time-Format?

    11.2 Product Structure

• Table 4 is missing the axisNames for "LEVEL 2 CONTENT" for QualityOfBathymetryCoverage see Figure 10

  11.2.3 BathymetryCoverage and QualityOfBathymetryCoverage Tables (in Group_F)

• Table 6 QualityOfBathymetryCoverage is not valid
  • name -> not set -> either all values are unset or "(empty)" [name, uom.name, upper]
  • the code id must be available in the feature catalogue and die attribute in the FC must correspond to IHO registry
  • An Attribute id has not been defined at the feature QualityOfBathymetryCoverage in the IHO registry

    11.2.5 Feature Instance group — BathymetryCoverage.nn

• Table 9 No. 11 verticalDatumReference
  • This makes no sense. The concept is called "Attribute Overriding". An Attribute is only allowed to override if it changes.
  • This attribute won't change because it is limited by S-102 to the value "1".
• ~~Second last paragraph
  • "If multiple vertical datums are present [...] and verticalDatumReference (on the feature instance group)."
  • Only if verticalDatumReference differs from the root-group, which is never the case.

    Annex A.1.1 BathymetryCoverage

• The uncertainty attribute is mandatory, which is not correct. According to S-102 3.0.0 clause 11.2.7 the uncertainty is optional.

  Annex A.1.2 QualityOfBathymetryCoverage

• The listed attribute id is not registered within IHO registry for the feature QualityOfBathymetryCoverage

  Annex A.2 Feature Attributes

  • The attributes of QualityOfBathymetryCoverage are missing

[RohdeBSH]

FC

• needs to be updated
  • Attribute binding for feature QualityOfBathymetryCoverage is not correct
  • uses depth & uncertrainty instead of id

[RohdeBSH]

Review done

[RohdeBSH]

Im sorry here is another one.

11.2.3 BathymetryCoverage and QualityOfBathymetryCoverage Tables (in Group_F)

• Table 6 BathymetryCoverage.uncertainty
  • According to IHO Registry, the uncertainty Attribute associated with BathymetryCoverage has an incorrect range
  • IHO Reg: Lower=0 & Upper=unset
  • In my opinion, S-102 has to correct the upper to "(empty)" and the closure to geSemiInterval

[RohdeBSH]

Based on artefact from 2024-07-11T20:36:58Z

9.2.8 Root QualityOfBathymetryCoverage

• Table 11 - Elements of featureAttributeTable compound datatype
  • bathyCoverage is not registered in IHO GI Registry (see S-100 5.2.0 10c-9.6.2 & #118)
  • bathymetricUncertaintyType is not registered in IHO GI Registry (see S-100 5.2.0 10c-9.6.2 & #118)

[giumas]

@RohdeBSH what about collecting all the missing QualityOfBathymetryCoverage attributes in a single ticket?

We can then close https://github.com/iho-ohi/S-102-Product-Specification/issues/118 and https://github.com/iho-ohi/S-102-Product-Specification/issues/80

[rmalyankar]

PT16 approved a change to the name and definition of bathymetricUncertaintyType, see #78 and the minutes. It is in the registry as typeOfBathymetricEstimationUncertainty.

There is an open issue for bathyCoverage, which, to the best of my recollection, the project team has not yet discussed and resolved. #80

[RohdeBSH]

what about collecting all the missing QualityOfBathymetryCoverage attributes in a single ticket?

I'm fine with that. @giumas would you please close the issues and we continue in this one.

[RohdeBSH]

It is in the registry as typeOfBathymetricEstimationUncertainty.

@rmalyankar Perfect I will made the change in the document, thank you.

[giumas]

@RohdeBSH, in https://github.com/iho-ohi/S-102-Product-Specification/issues/118#issuecomment-2225057146 you have also mentioned the missing id attribute. If the comment is still valid, we should mention it here too.

[giumas]

Hi @RohdeBSH, @hasel001 and @CHS-LynnPatterson, would you mind providing some details on the "discussion with Lawrence and Lynn on 07/16/2024 1300UTC" that triggered https://github.com/iho-ohi/S-102-Product-Specification/commit/acd95cbbfd7c6d90348743adb9dcbb66ac0315b2 ?

The commit description mentions this ticket, but it is not obvious to me how the changes are linked to it!

[hasel001]

Hi @giumas ,

As you know, we are days from the delivery date for 3.0. After several sets of correspondence with Julia, Jeff, @rmalyankar, @RohdeBSH, and @CHS-LynnPatterson among others, we found that there was a fundamental discrepancy between S-100 and S-102 in their present versions.

@RohdeBSH can sum it up better, but essentially what we were calling id in QoBC is unregistered in the GI Registry and would further require a proposal to S-100 to make it workable whatsoever.

This late in the game, I think the only feasible solution (apart from delivering nothing, which is untenable from my perspective) is to:

1. Create a draft Ed. 3.1.0, which still contains QoBC.

2. Remove QoBC from 3.0.0.

3. (BSH) Propose the necessary changes to S-100 (presumably 5.2.1) that will allow consistency between it and S-102.

4. Publish 3.1.0 as soon as practicable.

I apologize for the oversight on my part that allowed us to get this far with an unworkable provision in our spec, and I am working to make it right.

Please let me know if you have any other questions. I'm sorry for blindsiding everyone with the change, but I only realized the severity today.

[anthonyklemm]

What would the expected timeline be for getting S-100 updated so we can push a 3.1?

[giumas]

What about re-using in 3.0 one of the existing index-type concepts?

For instance, sourceRegistryNumber:

I believe that it is less bad than fully removing QoB at this point in time, after that it was there for so long!

[anthonyklemm]

I worry that it will send the wrong message of the importance of QoBC to OEMs investing development resources to support v3.0 if we have had it in v2.2 for such a long time, and then it quietly disappears in v3.0. If there is a viable creative solution to keep QoBC in 3.0, I think we should consider it.

[RohdeBSH]

Hi, The problem is that the column “id” in the featureAttributeTable is prescribed as identification of the metadata record (table row). Unfortunately, the values data set (grid) of the QoBC does not have this specification. However, as the Values data set is a compound data set, the attributes in the grid must have a name. This name must be registered in the IHO Concept Register. This is currently not the case. The name in the Values dataset must also be described in the feature catalog and also in the group_F. The idea of the DataCodingFormat=9 was that there is a clear connection between the featureAttributeTable and the Values dataset specified by the S-100. Unfortunately, this is not described clearly enough in the current S-100.

Conclusion:

1. the IHO Concept Register requires the registration of an attribute for QoBC
2. “id” is prescribed for the featureAttributeTable as a link column by S-100
3. the attribute names for linking the Values data set and the featureAttributeTable should be identical
4. it is possible to use an already registered attribute for the IHO Concept Register (QoBC). However, this does not change point 1.
   1. Different names for the link attributes is also possible according to the unbiased reading of the S-100. However, this leads to increased implementation effort for the OEMs. As a data producer (with self-developed software), I don't care. I have a separate driver for S-102 anyway and don't have to support the different product specifications. Nevertheless, I do not consider different names to be architecturally correct.
   2. I don't think it's correct to force a solution in a hurry. This forced solution must then be legitimized by a correction of the S-100. I see the problem that we will then get into difficulties due to backwards compatibility of the IHO registry

The idea was to temporarily remove QoBC for ed 3.0.0, fix everything in the S-100 and IHO registry and then immediately put it back in for ed. 3.1.0. The proposals for the changes to the S-100 have already been prepared (but not sent) and can be discussed and approved at the November meeting of the S-100WG. The BSH firmly assumes that there will be an S-100 5.2.1 before 2026. The S-102 3.1.0 is already in the drawer and just needs to be taken out. We would not remove already written program code from our S-102 3.0.0 driver. We would only prevent the QoBC from being written, but even that is not really necessary. As the QoBC does not appear in the FC, it would not be read by the ECDIS systems anyway, even if it is included in a 3.0.0 product.

Personally, I don't think that sends the wrong message to the OEMs or the shipping industry either. I think it sends the message that we are not acting quick and dirty, but thoughtfully and correctly. And that we are doing everything we can not to jeopardize the safety of shipping and to provide the best possible data.

Image added:

[giumas]

I warmly recommend keeping the QoB in 3.0, with the current 'id' (interpreting it as the indices of an enumeration) or, if such an interpretation is not judged valid, adopting one of the existing index-like concepts.

I suggest going with option 1 and just use 'id'. In the meantime, we start the registration process so that any potential interpretation issues will be fixed when 'id' is registered.

Added: I believe that the 'id' in QoB is more similar to the integer values used in enumeration like, for instance, S-57's TECSOU:

If we don’t need to register a techniqueOfSoundingMeasurementId nor an attributeId, why should we have a qualityOfBathymetryId or a featureAttributeTableIdx?

[hasel001]

I am working on our submission for Ed. 3.0.0, and this issue seems to be the largest collection of items (possibly) needing resolution. The following items (with corresponding statuses) are what I have collected from the prior posts in this thread. If you have more information on any of the items, or if you see any other glaring issues, please post it here. Once I shoehorn everything into the proper format and order, I will post a draft of the document here as well.

• [x] Section 1 - Formerly unresolved reference to S-100 Ed. 5.2.0 (resolved)
• [x] Section 4 - SequenceRule has been replaced by sequenceType (I believe this one has been fixed, but could @RohdeBSH please confirm?)
• [x] Overall - Overview appears at end (Still remains; I will manually reorder and modify ToC as necessary, etc., in submitted copy.)
• [ ] Section 5 - Temporal Reference System (@RohdeBSH I read your comment about time formats. Was it just a question, or is there an action I need to take?)
• [x] Section 10 - name now has (empty), making it parallel with upper and uom.name
• [x] FC - (it has been updated with what Yong sent on 11 July)
• [x] Annex A - attributes of QoBC not defined (Which ones should be listed here?)
• [x] Section 10 - Root QualityOfBathymetryCoverage: bathyCoverage is not registered (Can we leave it in the document and submit a proposal to register as soon as possible? It seems we could likely get it registered by the time the spec goes before the Member States.)

[hasel001]

Here is the promised draft version. There are some rather crude formatting hacks I had to employ to get things appearing in the correct way, but I'm told Metanorma will be fixed in about 6 days. And in any case, it proves the concept that it can be made to work if we have to push it out before that.

[RohdeBSH]

• Section 4 - SequenceRule has been replaced by sequenceType (I believe this one has been fixed, but could @RohdeBSH please confirm?)

Not really, now it is redundant to the CV_SequenceRule chapter. But it fits for now...

• Section 5 - Temporal Reference System (@RohdeBSH I read your comment about time formats. Was it just a question, or is there an action I need to take?)

DateTime and Time are two different data types. DateTime must be in UTC according to the statement. The statement does not exist for Time. In addition, there is only one DateTime attribute (TimePoint) in the S-102 and this is assigned with a default value. For me, this means that I can continue to specify the IssueTime in localTime. The question is whether this was intended. In my opinion, all time specifications should be in UTC. That was the point here. It is not wrong what is written in the S-102, it just allows different interpretations and I don't like that. As of 3.0.0 the BSH specifies all dates and times in UTC, including the IssueTime (e.g. 102600Z)

• Annex A - attributes of QoBC not defined (Which ones should be listed here?)

The registered "id".

• Section 10 - Root QualityOfBathymetryCoverage: bathyCoverage is not registered (Can we leave it in the document and submit a proposal to register as soon as possible? It seems we could likely get it registered by the time the spec goes before the Member States.)

It's dirty but okay for now ;)

[RohdeBSH]

• Section 10 - name now has (empty), making it parallel with upper and uom.name

This depends on what has been registered for "id".

[CHS-LynnPatterson]

Here is what was registered for ID

From: RohdeBSH @.> Sent: Thursday, July 18, 2024 6:41 AM To: iho-ohi/S-102-Product-Specification @.> Cc: Patterson, Lynn (DFO/MPO) @.>; Mention @.> Subject: Re: [iho-ohi/S-102-Product-Specification] Review - 3.0.0 - BSH findings (Issue #115)

• Section 10 - name now has (empty), making it parallel with upper and uom.name

This depends on what has been registered for "id".

— Reply to this email directly, view it on GitHubhttps://github.com/iho-ohi/S-102-Product-Specification/issues/115#issuecomment-2236073725, or unsubscribehttps://github.com/notifications/unsubscribe-auth/A54RIRC3L3JEDGSTRHDOTYLZM6ES3AVCNFSM6AAAAABKM7UEO2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZWGA3TGNZSGU. You are receiving this because you were mentioned.Message ID: @.**@.>>

[giumas]

Section 10 - Root QualityOfBathymetryCoverage: bathyCoverage is not registered (Can we leave it in the document and submit a proposal to register as soon as possible? It seems we could likely get it registered by the time the spec goes before the Member States.)

As mentioned in https://github.com/iho-ohi/S-102-Product-Specification/issues/80#issuecomment-2132181190, there is naming inconsistency between 'bathyCoverage' and 'fullSeafloorCoverageAchieved'. Given that they are used in combination, the 'Achieved' should be present or absent in both names.

[hasel001]

@giumas I got half way through writing a detailed argument for why I thought coverage didn't need achieved but fullSeafloorCoverage did. As I was consulting Ed. 6 of S-44, I found usage that convinced me that they both need it. As long as we have to register it anyway, I am in favor of making the name bathyCoverageAchieved and registering it as such (based on the desire to maintain consistency in language with S-44).

[hasel001]

Here is the latest draft copy with all changes incorporated (including my response to @guimas 53 minutes or so ago). Please let me know if you see any glaring issues with this one (based on the content). Otherwise it is what I plan to submit to Julia tomorrow evening UTC -5. As I mentioned before, Metanorma will be fixed early next week, so I will replace this submission with one that is less kluged together, but the content should essentially be the same.

Thank you to everyone for your time, your assistance, your patience, and your support. I feel a great deal of pride and satisfaction that we are about to cross this major threshold as a Project Team, and I am humbled as always to have the honor to serve with all of you. Thank you for everything you do and will continue to do.

[hasel001]

Ed. 3.0.0 has been submitted for S-100WG review, so I am closing this issue. If there are aspects of this issue that require further discussion, please repost as new issues (or in other existing issues, as the case may be).