Add rules according to notes in the documentation

[ClemensLinnhoff]

Reference to a related issue in the repository

#805

Add a description

Added rules according to existing notes in the documentation.

Take this checklist as orientation for yourself, if this PR is ready for the Change Control Board:

• My suggestion follows the style and contributors guidelines.
• I have taken care about the message documentation and the fields and enums documentation.
• I have done the DCO signoff.
• My changes generate no errors when passing CI tests.
• I have successfully implemented and tested my fix/feature locally.
• Appropriate reviewer(s) are assigned.

[pmai]

Looks good otherwise, assuming that the added rules have been checked to work in the validator.

[pmai]

I think this is not correct, as globally unique requires the ID to be unique across all messages of all types, whereas the current requirement is just uniqueness within all route messages (to one TP), which also makes more sense.

So the validator would need to properly handle a rule that checks for this kind of scoped uniqueness, which it currently does not; hence

Suggested change

// is_globally_unique

[ClemensLinnhoff]

@pmai I was too quick in committing this. On second thought: Why would anything be unique across all messages of all types? If this is really the intention, we already have a problem: moving object ID in the ground truth is set as is_globally_unique, as well as sensor id in the sensor_view. Does your interpretation mean, that if I have a sensor_id = 1 I cannot have a moving object with id = 1? This does not make sense to me.

So in my interpretation is_globally_unique means, that the uniqueness is related to that specific field. Either way, we definitely need a section in the documentation describing the rules to avoid these different interpretations.

[pmai]

It means, as you say, that all of the globally unique IDs must be unique across all messages/fields, as also implemented in the osi-validation rule implementation. I agree that for sensor_id it probably does not make sense, but it stems from the fact that we have moving object, stationary object, traffic signs and traffic lights, etc., and initially it was thought that it would be better to keep all of these IDs non-overlapping, hence the is_globally_unique rule (also ask @jdsika for the reasoning/discussions at the time).

This probably still makes sense, especially as there are still plans to merge those classes at some time. However for other ID fields (especially newly introduced ones), something more strict makes more sense. But we have no rule for this as of yet.

[ClemensLinnhoff]

Okay, I understand. But then we should remove globally unique from all non-object IDs, such as sensor ID. I can make the change here.

[ClemensLinnhoff]

Looks good otherwise, assuming that the added rules have been checked to work in the validator.

Yes, I checked that the rules are correctly parsed into the yml files by rules2yml of osi-validation.

[pmai]

CCB 2024-05-06: Merge as-is.

Potentially further rule additions that are direct result of existing textual rules in the release could be added in maintenance/patch releases, depending on our future approach to maintenance and maintenance releases.

[ClemensLinnhoff]

CCB 2024-05-06: Merge as-is.

Potentially further rule additions that are direct result of existing textual rules in the release could be added in maintenance/patch releases, depending on our future approach to maintenance and maintenance releases.

Before you megre, should we remove is_globally_unique from the sensor_ids then? Or is this considered a breaking change since the rule already existed in 3.6?

[PhRosenberger]

Before you megre, should we remove is_globally_unique from the sensor_ids then? Or is this considered a breaking change since the rule already existed in 3.6?

Let's not rush this and discuss the rules that are already there in a separate issue/PR. We are not sure if this is a quick fix for a patch or if it needs further discussion..

[ClemensLinnhoff]

Before you megre, should we remove is_globally_unique from the sensor_ids then? Or is this considered a breaking change since the rule already existed in 3.6?

Let's not rush this and discuss the rules that are already there in a separate issue/PR. We are not sure if this is a quick fix for a patch or if it needs further discussion..

But if it is the way @pmai describes, a lot of trace files will definitely fail during validation. It is quite common to have sensor_id set to 0 und the host vehicle also having ID 0. I would consider the rule is_globally_unique in the sensor_id a bug.

[ClemensLinnhoff]

Another thing: This is the definition of the Identifier in the documentation:

Has to be unique among all simulated items at any given time. For ground truth, the identifier of an item (object, lane, sign, etc.) must remain stable over its lifetime. Identifier values may be only be reused if the available address space is exhausted and the specific values have not been in use for several time steps. Sensor specific tracking IDs have no restrictions and should behave according to the sensor specifications.

Here it says, everything shall be unique except for Sensor specific tracking IDs.

I propose:

• Set rule is_globally_unique in all fields that are actually supposed to be globally unique (NOT sensor ID or tracking ID for example).
• Change description of Identifier to state that the ID is only globally unique, if specified by the rule is_globally_unique.

As already stated: I would consider this a bug in the documentation and therefore would fix this before the 3.7 release.

[ClemensLinnhoff]

Since my previous comment has been ignored and this PR has been just merged, I have created a new issue for the documentation bug: #809