simulation history sampling is counterintuitive

[mh0797]

Describe the bug

The simulation history sampling can counterintuitive. This can be best explained considering the following example.

Example

Assume a model with past_trajectory_sampling.time_horizon=1.9 and past_trajectory_sampling.num_poses=4.
(Note, setting past_trajectory_sampling.time_horizon=2.0 is not possible, as described in this issue.
Ideally, the sampled past poses should be at the following time steps: [-0.475s, -0.95s, -1.425s, -1.9s]. However, as the logs themselves are recorded with a fixed sample interval of 0.1s, these time steps are between the recorded frames.

Expected Behavior

In my opinion the following behavior would be intuitive:
The time steps between the recorded frames are rounded to the nearest frame resulting in the following time steps: [-0.5s, -1.0s, -1.4s, -1.9s]. That way, the returned frames match the intended trajectory sampling as good as possible.

Actual Behavior

However, the following time steps are actually sampled: [-0.4s, -0.8s, -1.2s, -1.6s]. Apparently, this can drastically impact the performance of a planner that was trained with a past interval of 2.0s, as it will implicitly assume that the frames passed to it in simulation are sampled with the same interval.

Explanation

In simulation, the feature builders have access to the SimulationHistoryBuffer which contains a list of past observations and ego states. In order to extract the frames that correspond to the feature builders past trajectory sampling from this list, this function is used.
This function used a fixed step size for all time steps which is calculated from the desired past horizon and the number of samples and then rounded down (see here). In the above example, this would result in 0.475s being rounded down to 0.4s.

Takeaway

While I see that a fixed sample interval may make sense, I suggest to reconsider this decision as it may have undesired side effects such as described in the example above. Maybe it would also be ok, to raise an error or a warning if the user sets the past_trajectory_sampling in a way that will not be applicable in simulation

Workaround

In order to not be affected by this issue, the following must hold for the past_trajectory_sampling
past_trajectory_sampling.time_horizon = k * past_trajectory_sampling.num_poses * 0.1s,
where k is a natural number. Also, past_trajectory_sampling.time_horizon must not exceed 1.9s as described in this issue.

[patk-motional]

Hi @mh0797,

Thank you for the analysis. You are indeed correct. For the challenge release, we will fix this issue by making sure that the buffer is at least 2s long. For example, 22 steps at 0.1s intervals should yield ~2.1s