iamlab-cmu / isaacgym-utils

Wrappers and utilities for Nvidia IsaacGym
Apache License 2.0
90 stars 19 forks source link

Add stable_baselines3 dependency #22

Closed tabula-rosa closed 3 years ago

tabula-rosa commented 3 years ago

This PR adds stable_baselines3 to the isaacgym-utils requirements.

Without this, both python examples/franka_rl_vec_env.py and python examples/franka_rl_stable_baselines.py would fail with error: (ModuleNotFoundError: No module named 'stable_baselines3')

Now, python examples/franka_rl_vec_env.py works successfully.

However, python examples/franka_rl_stable_baselines.py still throws an error, but for a different reason that I will open an Issue for. (TypeError: Can't instantiate abstract class GymFrankaBlockVecEnvStableBaselines with abstract methods env_is_wrapped)

jacky-liang commented 3 years ago

I'm of the opinion that stable_baselines or stable_baselines3 shouldn't be added to the requirements, since they're not required to use the simulation. Adding them to the requirements means when pip install happens those packages will be added, and as a consequence pytorch or tf will be installed, which seems undesirable since people who are installing a simulation probably wasn't thinking about installing NN libraries.

tabula-rosa commented 3 years ago

I understand the concern about adding something if it's not required. Would you be open to doing something like gym does where you would specify gym[all] for everything or what you need and gym is a minimal version?

Note that isaacgym brings in torch already as it is a requirement. It does not bring in tensorflow. This is my pip list after only installing isaacgym:

Package           Version  Location
----------------- -------- ------------------------------
cloudpickle       1.6.0
gym               0.19.0
imageio           2.9.0
isaacgym          1.0rc2   /home/telee/ws/isaacgym/python
ninja             1.10.2
numpy             1.21.2
Pillow            8.3.1
pip               21.2.4
PyYAML            5.4.1
rl-pytorch        1.0.1
scipy             1.7.1
setuptools        57.4.0
torch             1.9.0
torchvision       0.10.0
typing-extensions 3.10.0.0
wheel             0.37.0

I compared my pip list versions before this PR and using this PR. Adding stable_baselines3 as a dependency right now only introduces two new packages: pandas and stable-baselines3. So pip install stable_baselines3 does not bring in tensorflow, at least, with my computer setup.

(deleteme_wo_sb) telee@iam-ronan:~/ws/isaacgym-utils$ diff pip_list_after_igu_wo_sb.log pip_list_after_igu_w_sb.log 
36a37
> pandas             1.3.2
63a65
> stable-baselines3  1.1.0
tabula-rosa commented 3 years ago

I see your point about not wanting to bring in TensorFlow, and I don't think I would want to bring it in either. But it seems like stable baselines 3 is build around PyTorch and not TensorFlow. However, I haven't yet used SB3, so I can't confirm, but it appears that way. https://stable-baselines3.readthedocs.io/en/master/guide/migration.html

jacky-liang commented 3 years ago

Sure [all] sounds good lets just do that

tabula-rosa commented 3 years ago

Okay, I pushed the code and updated the README to indicate that users can pass [rl] for reinforcement learning (which pulls in stable_baselines3 or [all] for everything. Right now [all] is just RL, but it provides the means to capture future optional groups.