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

FC: Collective multiplicities not specified correctly #140

Open
DavidGrant-NIWC opened this issue Mar 21, 2024 · 7 comments
Open

FC: Collective multiplicities not specified correctly #140

DavidGrant-NIWC opened this issue Mar 21, 2024 · 7 comments

Comments

@DavidGrant-NIWC
Copy link

Collective multiplicities are not specified correctly. For instance, FairwaySystem has an AidsToNavigationAssociation which uses a collective multiplicity (there are multiple feature types in the binding) and the lower bound of the collective multiplicity is zero (indicating that there doesn’t have to be an aid associated with the FairwaySystem):
image

The DCEG shows this as requiring that one of the possible features must be present (the lower bound of the collective multiplicity is always one, indicating that an aid must be associated with the FairwaySystem):
image

This issue seems to be present for every binding which uses a collective multiplicity.

@TDYCARHugh
Copy link

I agree but...
Some of them had a minimum of 1 before and there was push back (not from me) that during production or editing the collection might be empty at times (during creation or swapping objects in the collection). The discussion, as I understood it, was that we should implement the collective minimums through validation checks. I think the issue might have been more focused on whether an equipment object must have a structure or not and then it was carried through to collections as well.

Either way is ok but I usually prefer to make a tighter model instead of adding more validation checks.

@DavidGrant-NIWC
Copy link
Author

Some of them had a minimum of 1 before and there was push back (not from me) that during production or editing the collection might be empty at times (during creation or swapping objects in the collection).

I get that you aren't advocating for this, but for those that are, note that the collection is only expected to satisfy these constraints after editing / production is completed. To include these items in a collection and then set their lower multiplicity to zero doesn't make much sense; why are they in a collection at all?

@TDYCARHugh
Copy link

I think the collections should have a min of 1 member. I was paraphrasing the arguments for why they got changed to 0, which did not come from me. I don't remember if it was another Github issue or an email thread where that discussion happened.

@kusala9
Copy link

kusala9 commented Mar 22, 2024

I'm probably the source of that :-) I noticed the vast preponderance of mandatory associations and asked for them to be taken out so they match what is required from the DCEG. My understanding, having spoken to Jeff at TSM (and before) was that the conditional multiplicity wasn't mandatory and the only mandatory associations were things like IslandGroup and TextPlacement where they make no sense to exist on their own. However, I agree that where an association is mandated by S-58 and we can replicate it in the S-101 FC we should probably try and do so. But, this could create a large overhead for producers if the associations either don't exist or aren't convertible in the source S-57 (but I would have thought they should be?). There's a riusk though that we introduce 100s of validation errors in test data.

I was at the IC-ENC conference with Jeff this week and he showed me the revisions to the associations he's done for the DCEG and I'm not sure the mandatory/optional question is answered yet, but I think we're starting from the point where most are still optional. Maybe we should go through those mandated from S-58 and try to come up with a definitive list. Now the naming convention is agreed upon it would be good to do this before June PT???

@kusala9
Copy link

kusala9 commented Mar 22, 2024

previous issue on multiplicities is here:

#114

@gorogara
Copy link

gorogara commented Jul 8, 2024

@DavidGrant-NIWC @kusala9 @TDYCARHugh @JeffWootton

Currently, all Collective Multiplicity have been removed from the DCEG.
Would it be correct to maintain the S100Base:lower value at 0?
image

@DavidGrant-NIWC
Copy link
Author

The problem originates in the DCEG, you would need @JeffWootton to update the tables.

See https://iho.int/en/s-100wg6-2022 paper and presentation 4.26

image
image

When a binding has multiple targets, the multiplicity applies to the collection. So, the following binding indicates that a BridgeAggregation is described by the relationship between a Bridge and the targets: SpanFixed, SpanOpening, Pontoon, and/or PylonBridgeSupport. The FC indicates that zero or more of the targets must be present.

image

The FC is accurately modeling the information from the DCEG (which is wrong):
image

The proper way to describe this relationship is via separate bindings:

Feature Association Multiplicity Target(s)
Bridge BridgeAggregation 1:∞ SpanFixed, SpanOpening
Bridge BridgeAggregation 0:∞ Pontoon, PylonBridgeSupport

or alternatively:

Feature Association Multiplicity Target(s)
Bridge BridgeAggregation 1:∞ SpanFixed, SpanOpening
Bridge BridgeAggregation 0:∞ Pontoon
Bridge BridgeAggregation 0:∞ PylonBridgeSupport

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

5 participants
@kusala9 @gorogara @TDYCARHugh @DavidGrant-NIWC and others