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

Why was support for Tuple spaces removed completely? #730

Closed
4 tasks done
michalgregor opened this issue Aug 16, 2022 · 3 comments
Closed
4 tasks done

Why was support for Tuple spaces removed completely? #730

michalgregor opened this issue Aug 16, 2022 · 3 comments
Labels
question Further information is requested

Comments

@michalgregor
Copy link
Contributor

  • I have marked all applicable categories:
  • I have visited the source website
  • I have searched through the issue tracker for duplicates
  • I have mentioned version numbers, operating system and environment, where applicable:

Hi, everyone, it seems that this commit introduced the following check into BaseVectorEnv:

if isinstance(obs_list[0], tuple):
            raise TypeError(
                "Tuple observation space is not supported. ",
                "Please change it to array or dict space",
            )

I've seen that there was some discussion previously about Tuple spaces not really being a good fit for the way that Batch works. I am not sure, whether anything changed in this particular PR that precludes Tuple spaces from working altogether, but previously I was able to make them work (I've built a class that processes the batch into the correct format for the PyTorch module 🤷‍♂️).

If it is just that using them is considered messy / to be discouraged, maybe it's a bit extreme to raise an exception? Couldn't we turn it into a warning or something? Or is there a deeper reason behind the change?

@Trinkle23897 Trinkle23897 added the question Further information is requested label Aug 16, 2022
@Trinkle23897
Copy link
Collaborator

It's because if we use tuple action space together with Batch, there are tons of unpredict behaviors and can cause potential bugs. If we only give use a warning instead of explicitly raising error, it's hard for user to find out such kind of a bug.

@michalgregor
Copy link
Contributor Author

I see – does this really resolve the issue then though? You can still have Tuple spaces nested under a Dict space, no? That will not have the same problems?

@Trinkle23897
Copy link
Collaborator

No, we recommend to use nested Batch with numpy array instead of batch+tuple, since numpy array has type system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants