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

Improve determinism of PostUpdate callbacks using sensor data #2268

Open
scpeters opened this issue Dec 14, 2023 · 4 comments
Open

Improve determinism of PostUpdate callbacks using sensor data #2268

scpeters opened this issue Dec 14, 2023 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@scpeters
Copy link
Member

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
Copy link
Member Author

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
Copy link
Member Author

scpeters commented May 2, 2024

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 gazebosim/sdformat#1261 (comment)). 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
Copy link
Member Author

scpeters commented May 2, 2024

I've implemented a prototype in #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
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: To do
Development

No branches or pull requests

2 participants