-
Notifications
You must be signed in to change notification settings - Fork 273
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
Fixing missing number of steps check #435
Fixing missing number of steps check #435
Conversation
@pseudo-rnd-thoughts actually, do you have any thoughts about removing this value error and returning truncated=True? That way we don't need to wrap any Meta-World environment in a TimeLimit wrapper or keep track of the number of steps. |
@@ -459,6 +459,8 @@ def step(self, action): | |||
assert len(action) == 4, f"Actions should be size 4, got {len(action)}" | |||
self.set_xyz_action(action[:3]) | |||
self.do_simulation([action[-1], -action[-1]], n_frames=self.frame_skip) | |||
if getattr(self, 'curr_path_length', 0) > self.max_path_length: |
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.
Why are we using 'getattr' for this?
I'm a bit confused on the structure of the metaworld environments, is there a technical description anywhere? |
I copied the code that got removed from the modified MuJoco env to the Sawyer base class. We can remove this value error if we set truncated=True when the max number of steps has been hit? Unfortunately I don't think there's any great documentation on the environments themselves |
@pseudo-rnd-thoughts I removed the use of getattr |
15f269b
into
Farama-Foundation:master
In the original repo, the do_simulation function in the modified mujoco env had a check for the maximum number of steps per env. This was added back in now that Meta-World uses the Mujoco Env in Gymnasium