Update Python package with new ScenarI/O APIs #176

diegoferigo · 2020-04-15T10:55:44Z

This PR updates most of the Python resources to work with the new ScenarI/O APIs. In particular:

Removes the former Robot interfaces in favour of the new Model interface
- Remove Python Robot interfaces ceccd7f
- Remove GazeboRobot class a113c01
- Add new interfaces for Scenario Model classes 2325327
- Add new cartpole and pendulum examples 981285e
Remove PyBullet, it will be substituted by the new bullet backend of ignition physics as soon as it lands upstream
- Remove PyBulletRuntime 7658ea3
- Remove PyBulletRobot a113c01

What's more meaningful to review are the following items:

Update Task in e49366d. Now the task no longer exposes also the gym.Env interfaces. It's only the selected runtime that exposes it. This allows treating the runtime as environment and not anymore as wrapper, removing a forwarding step.
Update GazeboRuntime to use ScenarI/O bindings 70404c1

This PR is already quite heavy and I don't want to add anything else. The environments will be still composed of a Runtime and a Task, but now this is no longer enough. In fact, there will be a third component that populates the environments: the Randomizer (and they are implemented as environment wrappers that extend gym.Env.reset()). The introduction of randomizers was necessary to allow Tasks to operate on multiple models. The next PR will introduce them.

This reverts commit 8e23abc724268a10a8bce89268c156420949ee64.

They will be replaced by new proxy resources of the scenario::gazebo::Model class

It will be replace by the new bullet plugin of ignition physics

TODO: add the envs there

They are relay classes that wrap a scenario::base::Model class

diegoferigo · 2020-04-19T16:56:42Z

Ready to be reviewed

traversaro · 2020-04-19T17:53:28Z

cpp/scenario/gazebo/include/scenario/gazebo/utils.h

@@ -50,6 +50,7 @@ namespace scenario {
 std::string getModelFileFromFuel(const std::string& URI,
 const bool useCache = false);
 std::string getRandomString(const size_t length);
+ std::string getInstallPrefix();


I suggest to document that this is only valid if gym-ignition is installed in Developer mode.

Ok, I take advantage and I will document all the utils functions together in a new commit

Done in 9e8f58e

traversaro · 2020-04-19T18:01:03Z

cpp/scenario/gazebo/CMakeLists.txt

@@ -94,6 +94,11 @@ target_link_libraries(ScenarioGazebo
 set_target_properties(ScenarioGazebo PROPERTIES
 PUBLIC_HEADER "${SCENARIO_PUBLIC_HDRS}")

+if(NOT CMAKE_BUILD_TYPE STREQUAL "PyPI")


This will make life complicated for anyone packaging the project in any form, being it as part of a superbuild, as Debian packages or a conda recipe. Instead of conflating build logic and packaging logic, could we have an explicit CMake variable to select if the gym-ignition project get installed in User mode or Developer mode, and then just specify the default based on the CMAKE_BUILD_TYPE , something like:

if(NOT CMAKE_BUILD_TYPE STREQUAL "PyPI") set(GYM_IGNITION_DEVELOPER_MODE_DEFAULT ON) else() set(GYM_IGNITION_DEVELOPER_MODE_DEFAULT OFF) endif() option(GYM_IGNITION_DEVELOPER_MODE "If ON, gym-ignition is installed in Developer mode, otherwise in user mode" ${GYM_IGNITION_DEVELOPER_MODE_DEFAULT}) if(GYM_IGNITION_DEVELOPER_MODE) target_compile_options(ScenarioGazebo PRIVATE -DGYMIGNITION_CMAKE_INSTALL_PREFIX="${CMAKE_INSTALL_PREFIX}") endif()

Thanks for the suggestion of propagating the build type as normal option. I will have a look at it. I don't remember precisely why we had to define a new build type for PyPI in the first place, for sure I tried to use a normal CMake variable as first attempt and then I faced problems that led me to this build type workaround, that is much more convoluted. What I'm sure is that I didn't do it if there were more simple solutions :)

I am planning to give a refreshing also to the CMake project, I will keep you comment in mind.

This will make life complicated for anyone packaging the project in any form, being it as part of a superbuild, as Debian packages or a conda recipe.

This is something I don't understand. The PyPI type is not meant to be used by any downstream user or packager. It is meant to be used to create packages for PyPI, as the name (and we are the maintainers of the packages). Any other usage is not supported.

This will make life complicated for anyone packaging the project in any form, being it as part of a superbuild, as Debian packages or a conda recipe.

This is something I don't understand. The PyPI type is not meant to be used by any downstream user or packager. It is meant to be used to create packages for PyPI, as the name (and we are the maintainers of the packages). Any other usage is not supported.

Exactly. With this modification, now the only way to properly install gym-ignition in a relocatable way is to set the CMAKE_BUILD_TYPE to PyPI, and I guess that this is not supported if you are not installing for PyPI. I guess we want to support "proper" installation also if the project is not installed with PyPI .

Ow ok now I understood your initial point: supporting a relocatable installation which could be implemented in the same way we package the project for PyPI.

Note the using PyPI for this purpose is wrong. In fact, due to how the shared libraries must be placed in the site_package folder, you need to have the following installation tree:

install_tree/ ├── gym_ignition │ ├── [...] │ ├── __init__.py │ └── plugins │ ├── libControllerRunner.so │ ├── libECMProvider.so │ ├── libJointController.so │ └── libPhysicsSystem.so ├── gym_ignition_environments │ ├── __init__.py │ └── [...] ├── libECMSingleton.so ├── scenario_bindings.py └── _scenario_bindings.so

As you can see, shared libraries are installed in the root of the CMAKE_INSTALL_PREFIX, and plugins inside gym_ignition/plugins. This is not what you want to obtain in a regular installation! In light of this, we should start thinking how to make coexist a (static) relocatable Developer installation with the PyPI build type without messing too much with the CMake project.

This is not what you want to obtain in a regular installation! In light of this, we should start thinking how to make coexist a (static) relocatable Developer installation with the PyPI build type without messing too much with the CMake project.

Yes, we can also have a separate issue for that.

raffaello-camoriano · 2020-04-25T00:46:13Z

gym_ignition/runtimes/gazebo_runtime.py

+ Gazebo simulator.
+
+ Args:
+ task_cls: The class of the handled task.


Which classes are supported?

raffaello-camoriano · 2020-04-25T00:59:09Z

gym_ignition/runtimes/gazebo_runtime.py

+ # Execute a paused step
+ ok_run = self.gazebo.run(paused=True)


I suggest to explain the meaning of and need for a paused step in this context somewhere in the documentation in the future.

Link for future #37

diegoferigo added 9 commits April 15, 2020 12:53

Revert "Disable gym_ignition package"

7b0b014

This reverts commit 8e23abc724268a10a8bce89268c156420949ee64.

Update Python files with new GazeboSimulator class

aaf3395

Remove robot interfaces from Python

ceccd7f

Remove gazebo robot in favor of ScenarI/O

a113c01

They will be replaced by new proxy resources of the scenario::gazebo::Model class

Move tasks to the new gym_ignition_environments package

d111c9a

Remove deprecated pybullet runtime

7658ea3

It will be replace by the new bullet plugin of ignition physics

Remove pybullet envs

5f802f0

Move environments to gym_ignition_environments

146e397

TODO: add the envs there

Move gympp_env to experimental

511a4c0

diegoferigo added complexity::medium pr::status::in-progress component::gym_ignition labels Apr 15, 2020

diegoferigo self-assigned this Apr 15, 2020

diegoferigo changed the title ~~Updat Python package with new ScenarI/O APIs~~ Update Python package with new ScenarI/O APIs Apr 15, 2020

diegoferigo force-pushed the refactor/python branch from a79a167 to 5197196 Compare April 15, 2020 12:24

diegoferigo added 4 commits April 15, 2020 14:41

Update base.task.Task

e49366d

Update base.runtime.Runtime

afe72d5

Update runtimes.gazebo_runtime

70404c1

Update runtimes.realtime_runtime

3981215

diegoferigo force-pushed the refactor/python branch from 5197196 to 8c51e7f Compare April 15, 2020 12:41

diegoferigo added 10 commits April 15, 2020 18:53

Update environment configuration

acba20e

Update utils.logger

2663f19

Remove utils.misc

6fbb653

Update utils.typing

9e08947

Expose ECMSingleton to Python

03e8a54

New utils.scenario helper module

7260696

Update main init file

4d6db91

Configure the ScenarI/O verbosity from Python

97241fc

Configure the environment in the package init file

889d455

Find by default resources from IGN_GAZEBO_RESOURCE_PATH

57338d1

diegoferigo added 2 commits April 15, 2020 18:53

Add new CartPoleContinuousSwingup task

db84c8e

Fix import loop

5ed481e

diegoferigo force-pushed the refactor/python branch from 111a769 to 3f4ba8a Compare April 15, 2020 16:53

diegoferigo added 5 commits April 16, 2020 08:56

Setup the environment variables for gym-ignition-models

6062f12

Store the seed in task.Task as public member

80f0db7

Add a contextmanager to change locally the verbosity

cf1743a

Helper to write a string to a tempfile

e632660

Update demo tasks

5ec561a

diegoferigo force-pushed the refactor/python branch from 3f4ba8a to 5ec561a Compare April 19, 2020 16:26

diegoferigo added 2 commits April 19, 2020 18:32

New interfaces for ScenarI/O models

2325327

New cartpole and pendulum Python models

981285e

They are relay classes that wrap a scenario::base::Model class

diegoferigo added issue::status::feedback and removed pr::status::in-progress labels Apr 19, 2020

diegoferigo requested review from traversaro, raffaello-camoriano and paolo-viceconte April 19, 2020 16:56

diegoferigo marked this pull request as ready for review April 19, 2020 16:58

traversaro reviewed Apr 19, 2020

View reviewed changes

traversaro approved these changes Apr 19, 2020

View reviewed changes

diegoferigo added 2 commits April 20, 2020 12:59

Remove pybullet dependency

4ad6ed9

Document utils.h

9e8f58e

diegoferigo merged commit 424d295 into robotology:refactoring Apr 20, 2020

diegoferigo deleted the refactor/python branch April 20, 2020 12:59

diegoferigo added pr::resolution::merged and removed issue::status::feedback labels Apr 20, 2020

This was referenced Apr 20, 2020

Add Randomizers: model and physics #177

Merged

C++ refactoring roadmap #155

Closed

raffaello-camoriano reviewed Apr 25, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Python package with new ScenarI/O APIs #176

Update Python package with new ScenarI/O APIs #176

diegoferigo commented Apr 15, 2020 •

edited

Loading

diegoferigo commented Apr 19, 2020

traversaro Apr 19, 2020

diegoferigo Apr 19, 2020

diegoferigo Apr 20, 2020

traversaro Apr 19, 2020

diegoferigo Apr 20, 2020 •

edited

Loading

traversaro Apr 20, 2020

diegoferigo Apr 20, 2020

traversaro Apr 20, 2020

raffaello-camoriano Apr 25, 2020

raffaello-camoriano Apr 25, 2020

diegoferigo May 19, 2020

		# Execute a paused step
		ok_run = self.gazebo.run(paused=True)

Update Python package with new ScenarI/O APIs #176

Update Python package with new ScenarI/O APIs #176

Conversation

diegoferigo commented Apr 15, 2020 • edited Loading

diegoferigo commented Apr 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegoferigo Apr 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diegoferigo commented Apr 15, 2020 •

edited

Loading

diegoferigo Apr 20, 2020 •

edited

Loading