Improve determinism of PostUpdate callbacks using sensor data

scpeters commented 9 months ago

Desired behavior

Currently many sensors are implemented with systems that publish new sensor data over gz-transport during PostUpdate, for example:

Since all PostUpdate code execution is performed in separate threads, a system that wanted to use the data from the current round of PostUpdate calls would have to do something like subscribing to the sensor's gz-transport topic and waiting for that topic to update or just use the most recently received value. In any case, this is not deterministic.

One good option would be to move some sensor updates from PostUpdate into the Update call, while ensuring that they are executed after the update of the Physics system. This ordering of update execution could be implemented using execution groups with numerical priority values that determine the order of execution (simiilar to the approach taken in rendering pipelines and init scripts). The sensor data should also then be written to components in the ECM.

Alternatives considered

Implementation suggestion

Additional context

scpeters commented 5 months ago

One good option would be to move some sensor updates from PostUpdate into the Update call, while ensuring that they are executed after the update of the Physics system. This ordering of update execution could be implemented using execution groups with numerical priority values that determine the order of execution (simiilar to the approach taken in rendering pipelines and init scripts). The sensor data should also then be written to components in the ECM.

I just discussed this with @azeey, who recommended that we try to keep the sensor updates in PostUpdate so that they will still be executed in parallel threads, but still executed in separate groups.

We also discussed how to specify order of execution priority: via XML or hard-coded at build time or a combination thereof. To simplify matters, @azeey suggested that we start with configuration only via XML. Also, while you could want different priority values for each callback in a system, @azeey suggested that the first prototype start with a single priority value per system that will apply to all its Update callbacks.

scpeters commented 5 months ago

I think it would be elegant to be able to specify the system priority as a namespace XML attribute like gz:system_priority as shown below:

<plugin filename="gz-sim-this-system"
        name="gz::sim::systems::This"
        gz:system_priority="0"/>

Currently, however the custom attributes of a <plugin/> tag are not storable in sdf::Plugin or a gz::msgs::Plugin message (see https://github.com/gazebosim/sdformat/issues/1261#issuecomment-2089583343). In the short term, it is easier to specify priority as an XML element in the plugin contents like the following:

<plugin filename="gz-sim-this-system"
        name="gz::sim::systems::This">
  <gz:system_priority>0</gz:system_priority>
</plugin>

scpeters commented 5 months ago

I've implemented a prototype in https://github.com/gazebosim/gz-sim/pull/2394 to control the order of execution of System::Update callbacks, which can be extended to support other callbacks as well if we are happy with the approach

scpeters commented 2 months ago

I've retargeted the prototype for controlling the execution order of System::Update callbacks to main for Ionic, and added support for PreUpdate execution order as well. It is ready for review in https://github.com/gazebosim/gz-sim/pull/2487

gazebosim / gz-sim