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

Add support for simulation reset via a publicly callable API #2648

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

arjo129
Copy link
Contributor

@arjo129 arjo129 commented Oct 11, 2024

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸

🎉 New feature

Closes #

Summary

This allows us to reset simulations without having to call into gz-transport making the code more readable from an external API. Depends on #2647 and also provides a unit test.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Added example and/or tutorial
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

This PR adds support for the Reset API to the test fixture. As `TestFixture`
is one of the main ways one can get access to the ECM in python
when trying to write some scripts for Deep Reinforcement Learning I
realized that without `Reset` supported in the `TestFixture` API, end
users would have a very hard time using our python APIs (which are
actually quite nice). For reference I'm hacking a demo template here:

https://github.com/arjo129/gz_deep_rl_experiments/tree/ionic

Signed-off-by: Arjo Chakravarty <[email protected]>
This allows us to reset simulations without having to call into
gz-transport making the code more readable from an external API.
Depends on #2647

Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Arjo Chakravarty <[email protected]>
@@ -369,6 +369,9 @@ namespace gz
/// \brief Set the next step to be blocking and paused.
public: void SetNextStepAsBlockingPaused(const bool value);

/// \brief Reset the current simulation runner
public: void Reset(const bool all, const bool time, const bool model);
Copy link
Contributor

Choose a reason for hiding this comment

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

Document parameters.

@@ -338,6 +338,9 @@ namespace gz
/// \brief Stop the server. This will stop all running simulations.
public: void Stop();

/// \brief Reset all runners in this simulation
public: void ResetAll();
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to not offer a Reset function that takes a _worldIndex similar to the other APIs in this class?

Base automatically changed from arjo/feat/reset_test_fixture to main December 20, 2024 05:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In review
Development

Successfully merging this pull request may close these issues.

2 participants