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 Randomizers: model and physics #177

Merged

Conversation

diegoferigo
Copy link
Collaborator

@diegoferigo diegoferigo commented Apr 19, 2020

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:

This structure is comparable to what we used to have before. However, the env obtained from the gyn.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:

  • Randomize the model using the new 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.
  • Randomize the physics (at the moment we only support loading DART and randomizing gravity, but as soon as ign-physics will have more back-ends, the used physics engine and other exposed parameters could be randomized).

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.

@diegoferigo
Copy link
Collaborator Author

The SDFRandomizer was designed after the discussion of #41 (comment). It was inspired by ryanjulian/rllab#61 and rlworkgroup/garage#51. If you're curious about the used pattern, you can refer to Builder Pattern that produces a Fluent Interface.

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:

  • The randomizer can operate on multiple elements matching a XPath. This means that, for example, with a single configuration entry we can randomize with the same statistics the masses of all the links.
  • Novel Method::Additive to alter the variable with additive noise. This is useful in combination of a gaussian randomization since we can add noise with zero mean.
  • Novel force_positive method. Depending on the selected randomization method and distribution, it could happen that a randomized value becomes less than zero. In order to avoid seeing dragons e.g. with negative masses, this method clips randomized values to force them being positive.

@diegoferigo diegoferigo force-pushed the refactor/randomizers branch 4 times, most recently from 240d2ad to f6e7d78 Compare April 20, 2020 15:26
@diegoferigo diegoferigo marked this pull request as ready for review April 20, 2020 15:27
@diegoferigo diegoferigo force-pushed the refactor/randomizers branch from f6e7d78 to 1781356 Compare April 20, 2020 16:40
@raffaello-camoriano
Copy link
Collaborator

This structure is comparable to what we used to have before. However, the env obtained from the gyn.make factory, is not usable by default.

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 GazeboEnvRandomizer around the env produced by gym.make.
Is this correct? Would it be feasible to make the raw gym.make env compatible with the framework?

@diegoferigo
Copy link
Collaborator Author

in a use case without any need for randomization the user would be required anyways to use an extension wrapper GazeboEnvRandomizer around the env produced by gym.make.
Is this correct? Would it be feasible to make the raw gym.make env compatible with the framework?

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 World.insertModel(urdf, pose, name) is specific for Gazebo, and if used in the Task class it will make the task not work in a RealTimeRuntime.

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 Task.reset_task method is a great solution. In this case, a randomizer is not necessary. I always consider generic tasks in my explanations since we're aiming to runtime transparency, but maybe for many users it is not a requirement that strict. We should keep it in mind when writing the documentation.

@raffaello-camoriano
Copy link
Collaborator

in a use case without any need for randomization the user would be required anyways to use an extension wrapper GazeboEnvRandomizer around the env produced by gym.make.
Is this correct? Would it be feasible to make the raw gym.make env compatible with the framework?

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 World.insertModel(urdf, pose, name) is specific for Gazebo, and if used in the Task class it will make the task not work in a RealTimeRuntime.

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 Task.reset_task method is a great solution. In this case, a randomizer is not necessary.

I see, thanks.

I always consider generic tasks in my explanations since we're aiming to runtime transparency, but maybe for many users it is not a requirement that strict. We should keep it in mind when writing the documentation.

Agreed.

@abc.abstractmethod
def randomize_model(self) -> str:
"""
Randomize the model.
Copy link
Collaborator

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.

Copy link
Collaborator Author

@diegoferigo diegoferigo Apr 24, 2020

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.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, I see.

Copy link
Collaborator

@raffaello-camoriano raffaello-camoriano Apr 26, 2020

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.

Copy link
Collaborator Author

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:

https://github.com/robotology/gym-ignition/blob/b1ff4419743c71e49e457b42213170a6c0f2f415/tests/test_gym_ignition/test_sdf_randomizer.py#L215

In general, tests are a great piece of implicit documentation that many ignore.

python/gym_ignition/randomizers/model/sdf.py Outdated Show resolved Hide resolved
python/gym_ignition/randomizers/model/sdf.py Outdated Show resolved Hide resolved
python/gym_ignition/randomizers/model/sdf.py Outdated Show resolved Hide resolved
python/gym_ignition/randomizers/model/sdf.py Show resolved Hide resolved
Comment on lines 205 to 206
def insert(self, randomization_data) -> None:
self._randomizations.append(randomization_data)
Copy link
Collaborator

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?

Copy link
Collaborator Author

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

See documentation ed0c1cb.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants