Skip to content

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 <arjoc@intrinsic.ai>
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 <arjoc@intrinsic.ai>
Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
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.

Copy link
Contributor Author

@arjo129 arjo129 Jan 8, 2025

Choose a reason for hiding this comment

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

I've left model reset in here because we have some scaffolding for it but, I'm not sure if I should leave it in.

Base automatically changed from arjo/feat/reset_test_fixture to main December 20, 2024 05:06
@azeey
Copy link
Contributor

azeey commented Jan 7, 2025

@arjo129 any updates on this PR?

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
@arjo129
Copy link
Contributor Author

arjo129 commented Feb 5, 2025

This should be ready for review.

@azeey azeey self-requested a review February 5, 2025 15:04
Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
@arjo129 arjo129 requested a review from iche033 February 7, 2025 01:54
Copy link
Contributor

@iche033 iche033 left a comment

Choose a reason for hiding this comment

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

just minor comment on doc otherwise looks good.

Signed-off-by: Arjo Chakravarty <arjoc@intrinsic.ai>
@arjo129 arjo129 enabled auto-merge (squash) February 10, 2025 05:45
@arjo129 arjo129 merged commit 6b7df49 into main Feb 10, 2025
9 checks passed
@arjo129 arjo129 deleted the arjo/feat/server_reset_public_api branch February 10, 2025 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants