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

[Feature Request] TensorSpec is_in methods should check the dtype of val #793

Open
1 task done
riiswa opened this issue Jan 4, 2023 · 2 comments · May be fixed by #1952
Open
1 task done

[Feature Request] TensorSpec is_in methods should check the dtype of val #793

riiswa opened this issue Jan 4, 2023 · 2 comments · May be fixed by #1952
Assignees
Labels
enhancement New feature or request Good first issue A good way to start hacking torchrl!

Comments

@riiswa
Copy link
Contributor

riiswa commented Jan 4, 2023

Motivation

As mentioned in #783, we should check that is_in always checks the dtype.

Solution

Add self.dtype == val.dtype condition in the is_in methods. Should be very simple to fix, but we need to make sure that all tests pass.

Checklist

  • I have checked that there is no similar issue in the repo (required)
@riiswa riiswa added the enhancement New feature or request label Jan 4, 2023
@vmoens
Copy link
Contributor

vmoens commented Jan 4, 2023

Thanks for keeping track of this.
Let me know if you want to take care of it, otherwise I'll find someone else

@riiswa
Copy link
Contributor Author

riiswa commented Jan 4, 2023

I think this is a good first issue for someone that want to make his first contribution. What do you think about adding the good first issue label to this (and to the repo) ? Otherwise, I'll take care of it.

@vmoens vmoens added the Good first issue A good way to start hacking torchrl! label Jan 4, 2023
@namanxkumar namanxkumar linked a pull request Feb 22, 2024 that will close this issue
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Good first issue A good way to start hacking torchrl!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants