-
Notifications
You must be signed in to change notification settings - Fork 26
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 Randomizers: model and physics #177
Add Randomizers: model and physics #177
Conversation
8569b90
to
52d16f3
Compare
The The mujoco randomization is somehow more complicated than what we can do with SDF. In fact, mujoco models store most of their parameters in the attributes of the DOM, instead SDF always use elements. Here a list of additional features I implemented:
|
240d2ad
to
f6e7d78
Compare
f6e7d78
to
1781356
Compare
As far as I can tell from the above comment and from 4d47415#diff-64386c8ba5955b2a8256341650413e50, in a use case without any need for randomization the user would be required anyways to use an extension wrapper |
It is correct and it was a consequence of the redesign. If you want the wrapped Task class to be independent from Gazebo, using a randomizer that just inserts the model and performs no randomizations is the only solution. In fact, the method However, in the case of a Task class that's supposed to work only with the Gazebo runtime (i.e. it does not target real-time transparency), populating the world in the |
I see, thanks.
Agreed. |
@abc.abstractmethod | ||
def randomize_model(self) -> str: | ||
""" | ||
Randomize the model. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future documentation it would be very useful to have a list of all the randomizable quantities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given that you can access all the elements through XPath patterns, you can randomize everything in the SDF. The only constraint is that randomized quantities must be elements and not attributes. This was a design choice that matches SDF, in fact all relevant data are stored in elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Users may find it useful to refer to a set of examples. The cartpole ones are a great point to start with 4d47415.
In the future, it may be valuable to also provide an example involving a more complex robot like Panda or iCub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR introduces a test with a Panda:
In general, tests are a great piece of implicit documentation that many ignore.
def insert(self, randomization_data) -> None: | ||
self._randomizations.append(randomization_data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are the randomization configuration data appended? How is self._randomization
organized? Is it a list with a randomization_data
configuration object for each element to be randomized?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tip: always check the type hinting, they're there to help you understand the type of the variables even in a language that is not statically typed.
You will find that _randomizations
is a List[RandomizationData]
. It is used in the add
method of the *Builder
to insert the new randomization in a buffer of this class.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See documentation ed0c1cb.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, thanks.
This PR complements #176 and adds the missing piece used in the new Python package: Randomizers.
Before, the combination of Task + Runtime was enough to make an environment. Now it's no longer the case. Take as reference the following:
launch_cartpole.py
example.CartPoleDiscreteBalancing
task, same as before.GazeboRuntime
that works with the new ScenarI/O APIs.This structure is comparable to what we used to have before. However, the
env
obtained from thegyn.make
factory, is not usable by default.Randomizers are developed as environment wrappers, and they mainly extend the
gym.Env.reset
method. They will add the model used by the task in the world [1] and optionally perform one or more of the following steps:SDFRandomizer
(for example the dynamics parameters of the model - mass, inertia, joint friction, ... - and the friction with ground). See the test in 52d16f3 for an example.Two randomizers examples, one that does not apply any randomization and another that fully randomizes what's currently supported, can be seen in 4d47415.
[1] This was previously done by the runtime since it was the only setup we were supporting. Now the randomizer could add as many models as are required. For example, in manipulation environments they could reset the manipulator and the work area.