pytorch / rl

A modular, primitive-first, python-first PyTorch library for Reinforcement Learning.
https://pytorch.org/rl
MIT License
2.37k stars 315 forks source link

[BUG] Main branch conflicts with tensordict #1245

Closed smorad closed 1 year ago

smorad commented 1 year ago

Describe the bug

I'm unable to use DQN on the latest commit.

To Reproduce

conda create -n torchrl python
conda activate torchrl
pip install torch
git clone https://github.com/pytorch/rl
cd rl
pip install -e .
git rev-parse HEAD # dc78be1136db7f1bb6c9102cfb26b6e79f188e2                                                                                                                                                                                                                  

Now run a simple import

from torchrl.modules.tensordict_module.actors import QValueActor
Traceback (most recent call last):
  File "/Users/smorad/code/explore/test.py", line 1, in <module>
    from torchrl.modules.tensordict_module.actors import QValueActor
  File "/Users/smorad/code/explore/rl/torchrl/__init__.py", line 32, in <module>
    import torchrl.collectors
  File "/Users/smorad/code/explore/rl/torchrl/collectors/__init__.py", line 6, in <module>
    from .collectors import (
  File "/Users/smorad/code/explore/rl/torchrl/collectors/collectors.py", line 40, in <module>
    from torchrl.envs.common import EnvBase
  File "/Users/smorad/code/explore/rl/torchrl/envs/__init__.py", line 10, in <module>
    from .transforms import (
  File "/Users/smorad/code/explore/rl/torchrl/envs/transforms/__init__.py", line 6, in <module>
    from .r3m import R3MTransform
  File "/Users/smorad/code/explore/rl/torchrl/envs/transforms/r3m.py", line 19, in <module>
    from torchrl.envs.transforms.transforms import (
  File "/Users/smorad/code/explore/rl/torchrl/envs/transforms/transforms.py", line 18, in <module>
    from tensordict.utils import expand_as_right, unravel_keys
ImportError: cannot import name 'unravel_keys' from 'tensordict.utils' (/Users/smorad/miniforge3/envs/torchrl/lib/python3.11/site-packages/tensordict/utils.py)

Ok, perhaps we need to update tensordict to latest

git clone https://github.com/pytorch-labs/tensordict
cd tensordict
pip install -e .
git rev-parse HEAD # 55e5be159389a88f9b77147ace17a6b5ffad0f88
from torchrl.modules.tensordict_module.actors import QValueActor

Now we get a new error

  File "/Users/smorad/code/explore/test.py", line 1, in <module>
    from torchrl.modules.tensordict_module.actors import QValueActor
  File "/Users/smorad/code/explore/rl/torchrl/__init__.py", line 32, in <module>
    import torchrl.collectors
  File "/Users/smorad/code/explore/rl/torchrl/collectors/__init__.py", line 6, in <module>
    from .collectors import (
  File "/Users/smorad/code/explore/rl/torchrl/collectors/collectors.py", line 25, in <module>
    from tensordict.nn import TensorDictModule, TensorDictModuleBase
  File "/Users/smorad/code/explore/tensordict/tensordict/nn/__init__.py", line 6, in <module>
    from tensordict.nn.common import (
  File "/Users/smorad/code/explore/tensordict/tensordict/nn/common.py", line 17, in <module>
    from tensordict.nn.functional_modules import make_functional
  File "/Users/smorad/code/explore/tensordict/tensordict/nn/functional_modules.py", line 18, in <module>
    from tensordict.tensordict import is_tensor_collection, TensorDictBase
  File "/Users/smorad/code/explore/tensordict/tensordict/__init__.py", line 7, in <module>
    from tensordict.persistent import PersistentTensorDict
  File "/Users/smorad/code/explore/tensordict/tensordict/persistent.py", line 26, in <module>
    from tensordict import MemmapTensor
ImportError: cannot import name 'MemmapTensor' from 'tensordict' (unknown location)

System info

Describe the characteristic of your environment: macos

Checklist

vmoens commented 1 year ago

Got it let me have look

vmoens commented 1 year ago

You're running this from /Users/smorad/code/explore/ right? And this is where you install tensordict. So perhaps it's trying to import from tensordict and not tensordict/tensordict?

smorad commented 1 year ago

Oh you're totally right, shadowing the installed tensordict with the directory was causing the second error. Latest commits work with both! It might be worth setting the rl:main branch to track the tensordict:main branch as a dependency, rather than whatever is in pip. This way people can just pip install git+https://github.com/pytorch/rl and get a compatible tensordict install.

I'd normally use the pip version of torchrl but there has been lots of fixes/development since the last stable release!

vmoens commented 1 year ago

I'd normally use the pip version of torchrl but there has been lots of fixes/development since the last stable release!

Yep things are moving fast around here :)

You can always use the nightly pip install torchrl-nightly