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

Documentation of OSI timestamps #839

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

thomassedlmayer
Copy link
Contributor

@thomassedlmayer thomassedlmayer commented Oct 25, 2024

Initial draft to add a new documentation page about different types of timestamps in OSI and how they are related or not.

So far the content was mainly taken from the existing class documentation and reformulated/simplified/summarized.

Potential mistakes or misunderstandings have to be checked.

Closes #834.

Signed-off-by: Thomas Sedlmayer <tsedlmayer@pmsfit.de>
@thomassedlmayer thomassedlmayer added the Documentation Everything which impacts the quality of the documentation and guidelines. label Oct 25, 2024
@thomassedlmayer thomassedlmayer self-assigned this Oct 25, 2024
@jdsika jdsika requested a review from TimmRuppert November 8, 2024 12:58
@jdsika jdsika added this to the V3.7.1 milestone Nov 8, 2024
Copy link
Contributor

@ClemensLinnhoff ClemensLinnhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very good to me. I just have some minor remarks/questions.

`Simulation time` refers to the timestamp within a simulation environment.

**Definition of zero time of simulation time**
- Zero time is arbitrary but must be identical for all frames and corresponding messages (probably within one simulation).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can add the "[...] within one simulation here" as proposed.


**SensorData-specific timestamp**
- Point in time that the sensor data message becomes available to the rest of the system (i.e. the driving functions).
- Corresponds with the sending time / publish time.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have the terms sending time / publish time somewhere in the documentation? Otherwise we could remove this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sending time is used in the class reference documentation. Carlo suggested to replace it with publish time but there was no final decision on that yet, so I added both as options as this is still a draft PR.


**Other top-level message timestamps**
- GroundTruth, SensorView, TrafficCommand, TrafficUpdate, TrafficCommandUpdate, StreamingUpdate
- The Timestamp coincides both with the notional simulation time the data applies to and with the time it was sent / published.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does timing work in a closed-loop co-simulation anyways? I am thinking about the timestamp of the TrafficUpdate, that gets fed back into the simulation engine / scenario player.

Let's say the scenario player publishes a SensorView with the timestamp 0.1 s.
A sensor model gets that, simulates the sensor output, which takes 100 ms, so it outputs a SensorData with timestamp 0.2 s.
That data gets processed by an object detection and path planning, which outputs to the actuator control and vehicle dynamics at 0.3 s.
This would also have some latency, for simplicity let's say also 100 ms. So the TrafficUpdate sent to the scenario player then has the timestamp 0.4.

Is my understanding generally correct? I have not really worked with closed-loop simulation much.
Can co-simulation platforms work with this delay?
Should we include an example like this in this document?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it depends a lot on the simulation environment and how it's designed. You can do all sorts of things depending on what you are trying to do. But generally, I'd say that explaining how to do simulation time management is really beyond the scope of OSI and its documentation.

I'd rather keep this overview page small and concise. So far I have mainly copied and pasted existing definitions from the class reference documentation into this single document and tried to order/categorize it. The goal was to have a basis to reduce it to the essentials (e.g. extract similarities/differences of timestamps, identify missing definitions). The current state of this PR is just a draft, and nothing I want to merge as it is, as that just adds redundancy.

So I'd say the main point of a review right now would be to check if the categorization of timestamps is correct, if there are any timestamps missing or if their definitions are misleading/wrong/incomplete. We can then use this to update the definitions in the class reference documentation and reduce this section down to just explaining the different major categories/types of timestamps and making clear which fields belong to these categories. What do you think about this approach?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think this is a good approach. I agree that this page should be more of an overview and conceptual explanation. I just think that examples always help understanding definitions.

Copy link

@TimmRuppert TimmRuppert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also have just minor comprehension questions to make it clearer for the reader.

**Definition of zero time of simulation time**
- Zero time is arbitrary but must be identical for all frames and corresponding messages (probably within one simulation).
- Zero time does not need to coincide with the unix epoch.
- It is recommended to use the start time of the simulation as zero time.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- It is recommended to use the start time of the simulation as zero time.
- It is recommended to define the zero time as the actual clock time at the moment the simulation begins

I would make it more clear so everyone understands what is meant with a "start time"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would make it more clear so everyone understands what is meant with a "start time"

I think however using "actual clock time" would make this unclearer, as this suggests that there is some relationship to wall clock time or something else that one would consider an actual clock (rather than a simulated one). Since we are not assuming a real-time simulation, talking about actual clocks seems problematic.

Maybe something like:

Suggested change
- It is recommended to use the start time of the simulation as zero time.
- It is recommended to define zero time to coincide with the start time (initial time step) of the simulation, i.e. simulation time zero.

- It is recommended to use the start time of the simulation as zero time.

**SensorData-specific timestamp**
- Point in time that the sensor data message becomes available to the rest of the system (i.e. the driving functions).

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the example and the term "becoming available" is a bit misleading. Becoming available to the driving functions cannot really be the same timestamp which excludes but latency, right?

As an idea:

Suggested change
- Point in time that the sensor data message becomes available to the rest of the system (i.e. the driving functions).
- Represents the exact point in time when the sensor data is published to the rest of the system.
- Accounts for any internal processing latency within the sensor.
- Excludes delays caused by bus communication (e.g., latency after sensor output).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the example and the term "becoming available" is a bit misleading. Becoming available to the driving functions cannot really be the same timestamp which excludes but latency, right?

The specification currently does not define what becoming available means. This allows for use cases where the whole delay (including any communication delays) is modeled/included in the sensor model (which is useful for many simpler use cases, especially where there is no contention on the communication channel, and/or exact bus modelling has no perceived benefit). It also allows for use cases where the timestamp reflects the sending time, and latency/communication effects are handled outside the sensor model.

Not saying that this situation is ideal, i.e. it would likely be good to have some machine-readable indication which it is, but there is little we can do in pure clarification/maintenance releases.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, understood that we cannot change that much. But how does the following:

This allows for use cases where the whole delay (including any communication delays) is modeled/included in the sensor model

fit with the current specification of:

  • Excludes delays caused by bus communication (e.g., latency after sensor output).

or from the current SensorData timestamp doc:

Latencies of bus communications, etc., that occur after the sensor output have to be applied on top of this, if needed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, understood that we cannot change that much. But how does the following:

This allows for use cases where the whole delay (including any communication delays) is modeled/included in the sensor model

fit with the current specification of:

  • Excludes delays caused by bus communication (e.g., latency after sensor output).

That is not actually part of the current OSI specification, though, is it?

or from the current SensorData timestamp doc:

Latencies of bus communications, etc., that occur after the sensor output have to be applied on top of this, if needed.

This is, and one can read it as suggesting that bus communication latency is to be handled outside the sensor (which is the second use case above, and needed if one really wants to model bus communication contention, for example - though that seldomly happens at OSI level I would venture); however the if needed leaves the door open for use case 1, i.e. if a simplified communication delay is already included in the sensor model, obviously adding bus latency on top is not needed.

Not saying that I like the lack of clarity here, but on the other hand people will do what they want to anyway, and details at this level really have to be documented and coordinated between parties in any case, regardless of what we write here.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is not actually part of the current OSI specification, though, is it?

That is part of this PR. 3 lines below.

however the if needed leaves the door open for use case 1, i.e. if a simplified communication delay is already included in the sensor model, obviously adding bus latency on top is not needed.

I do not see how the "if needed" allows to additional include these in the timestamp field? I understand it that way: "If you need communication latency (like bus latency) you need to add that this latency in the consumer of your SensorData.timestamp"

but on the other hand people will do what they want to anyway

Is that not the motivation to write a good standard?

details at this level really have to be documented and coordinated between parties in any case

You are totally right! But I just started this thread about the "becoming available" not being accurate with the following latency mentioning in the text on this added documentation. I do not want to talk about different forms of latency and where they should be included (nonetheless, I still did that in the beginning of this particular comment ;) ).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Strictly excluding all communication delays is not the same as saying that communication delays after sensor output are not included or should be added on top. The changed wording would disallow modelling delays within a sensor model whereas the old wording does not. I have changed the original wording in this PR to make it more technical and precise and less exemplary, but maybe I've made it more misleading and it shouldn't be changed because minor wording changes result in major content changes.

To me, "becoming available" (together with the rest of the definition) means that from that point the message is potentially accessible by other (simulation) components without specifying what the timestamp exactly means reality-wise.

So currently the timestamp describes the point in time at which the sensor model component (=sender) has released the message to the outside system regardless of what has happened internally, if I understand that correctly. "Simulated publish time" could also be contained in a sensor model but the message itself becomes available to the rest of the (simulation) system later. The only restriction we can extract from the current specification is that the timestamp must not contain delays that occur after the sensor model output. Did I get this right?

I guess the question is whether it is necessary and/or possible to clarify the actual content of the definition without changing it? Maybe we should just collect issues we see with the current state for future minor/major releases (like adding an indication for the two valid use cases as Pierre suggested)?

(Side note, because I stumbled across this while thinking and researching about the different terms: Although it should be clear from the context, 'send time' might be a more appropriate term than 'sending time', as the latter is often understood as the duration of the transmission process rather than a single point in time.)

- The Timestamp coincides both with the notional simulation time the data applies to and with the time it was sent / published.
- No inherent latency for the corresponding data, as opposed to e.g. SensorData-specific timestamp.

== Vehicle-global synchronized time::

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must/should be using the same same time reference as Simulation time` right? If yes, should we make that more clear?

Copy link
Contributor Author

@thomassedlmayer thomassedlmayer Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if that is the case. I think there could be use cases where you want to have an independent vehicle time course (e.g. simulation of timing issues/failures). As long as the simulation environment can handle the differences, this would be fine IMO. If you don't want to simulate that, you can of course use the same time frame.

Edit: I noticed that in the section "Definition of zero time of simulation time" this then has to be considered.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But even for timing issues/failures this is still the same time reference/zero time? I am not saying it has to be the same stamp (which allows you to consider jitter etc.).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But even for timing issues/failures this is still the same time reference/zero time? I am not saying it has to be the same stamp (which allows you to consider jitter etc.).

Whether the vehicle-global synchronized time has any fixed relationship to either simulation time or simulated real-time is completely open: It is the job of the (simulated) vehicle to determine vehicle-global synchronized time, hence whether it has any relationship depends on things outside of OSI. The other times (simulation time and simulated real-time, which only differ by epoch, but are otherwise synchronous) are deemed ground truth. Everything in HostVehicleData/SensorData/... is completely up to vehicle systems, and can differ by arbitrary, potentially changing offsets and frequencies: Consider frequency drift in chrystals, complete loss of battery power, GPS spoofing, arbitrary time sync errors, ... This goes beyond communication jitter, i.e. offsets can be arbitrarily large.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I think I made it not clear or do not fully understand the standard. I do not mean that this must be the same value as the step value of the current simulated time. I can be a different value to all kinds of reasons you mentioned but it must refer to the same zero time?

On this page here SensorData-specific timestamp is mentioned below the Simulation time. This includes internal latency etc. Then why is e.g. DetectedEntityHeader/measurement_time something that is vehicle-global synchronized - or why is SensorData-specific timestamp not? Why the split in the documentation?

The current class documentation of DetectedEntityHeader/measurement_time even says

... in the global synchronized time.

an not

... in the vehicle-global synchronized time.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I think I made it not clear or do not fully understand the standard. I do not mean that this must be the same value as the step value of the current simulated time. I can be a different value to all kinds of reasons you mentioned but it must refer to the same zero time?

It cannot refer to the same zero time, as that relationship can change over time: Simulated time proceeds in uniform fashion, whereas whatever the vehicle thinks its vehicle-global synchronized time is can change all of the time - consider resync with GPS clock after some frequency drift without GPS clock connection.

Or in other words: Simulation time (based on zero time), simulated real-time, etc. are under the simulations control and proceed with full regularity (ignoring leap seconds for now). Vehicle-glocal synchronized time is whatever the vehicle systems decide it is - it has nothing to do with the simulation, but all with the vehicle software and its interactions with simulated reality.

On this page here SensorData-specific timestamp is mentioned below the Simulation time. This includes internal latency etc. Then why is e.g. DetectedEntityHeader/measurement_time something that is vehicle-global synchronized - or why is SensorData-specific timestamp not? Why the split in the documentation?

Because SensorData.timestamp is for use by the simulation to ensure simulated timing is correct (it is not something that exists in the real world), whereas DetectedEntityHeader is purely for the vehicle system consumption (it is payload that exists in the real world), we do not care what it is from the point of view of the simulation.

They are two very different things.

The current class documentation of DetectedEntityHeader/measurement_time even says

... in the global synchronized time.

an not

... in the vehicle-global synchronized time.

Probably an editing oversight, as global synchronized time is supposed to be the vehicle's global synchronized time (might have happened during integration of the ISO 23150 wording).

- LogicalDetectionDataHeader/logical_detection_time
- Timestamp at which the transformation and optional fusion was finished in the vehicle-global synchronized time.

== Other timestamps
Copy link
Contributor Author

@thomassedlmayer thomassedlmayer Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially add definition of "simulated calendar time" if this is added in the mcap trace file format specification (#841).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Everything which impacts the quality of the documentation and guidelines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation page for timing
5 participants