OpenSimulationInterface / open-simulation-interface

A generic interface for the environmental perception of automated driving functions in virtual scenarios.
Other
265 stars 124 forks source link

Add rules according to notes in the documentation #806

Closed ClemensLinnhoff closed 2 months ago

ClemensLinnhoff commented 2 months ago

#### 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:

ClemensLinnhoff commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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 commented 2 months ago

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:

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

ClemensLinnhoff commented 2 months ago

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