-
Notifications
You must be signed in to change notification settings - Fork 418
[Feature] Brax envs #722
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] Brax envs #722
Conversation
Codecov Report
@@ Coverage Diff @@
## main #722 +/- ##
==========================================
+ Coverage 88.72% 88.79% +0.06%
==========================================
Files 120 122 +2
Lines 20241 20492 +251
==========================================
+ Hits 17958 18195 +237
- Misses 2283 2297 +14
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
vmoens
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's set up the test pipeline, I'll give it a second look once it's done
| >>> env = brax.envs.get_environment("ant") | ||
| >>> env = BraxWrapper(env) | ||
| >>> td = env.rand_step() | ||
| >>> print(td) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should put what the print output is here below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
torchrl/envs/libs/brax.py
Outdated
| NdUnboundedContinuousTensorSpec, | ||
| ) | ||
| from torchrl.envs.common import _EnvWrapper | ||
| from torchrl.envs.libs.jumanji import _torchrl_data_to_spec_transform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should definitely not import stuff from lib to lib.
Here I would put _torchrl_data_to_spec_transform in a utils file, jax_utils (and rename it to a less generic name).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
|
The tests are passing! Just a few edits and we're good to go! |
vmoens
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Let's wait for the tests to pass and merge this!
Description
Draft PR of Brax envs