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 is Frame_skip for gymnasium mujoco environments set to zero? #94

Closed
HiddeLekanne opened this issue Sep 15, 2023 · 3 comments · Fixed by #99
Closed

Why is Frame_skip for gymnasium mujoco environments set to zero? #94

HiddeLekanne opened this issue Sep 15, 2023 · 3 comments · Fixed by #99

Comments

@HiddeLekanne
Copy link

The default for these environments differs from environment to environment. If you set them all to zero, results become silently incomparable with other benchmarks which use the default values.

Also action_repeat = X is not necessarily equal to frame_skip = X. Mujoco Implementations use fixed step ODEs to solve their environments, meaning larger frame_skips result in larger (less accurate) steps.

@belerico
Copy link
Member

Hi @HiddeLekanne, it was a leftover from a previous implementation: thanks for spotting out!
Regarding the frame_skip vs action_repeat:

  • for mujoco (which can be instantiated with the sheeprl/configs/env/gym.yaml config changing the env.id field) we do not apply any frame_skip by default (w/o considering the frame_skip=0 leftover)
  • for dmc we have the frame_skip parameter and I think this can be misleading: we can remove that parameter and if one wants to apply the action_repeat it will set the env.action_repeat>=1 field

@HiddeLekanne
Copy link
Author

Ah I was just worried it was a design decision, and that the idea was to replace any frame_skip parameter with the action_repeat parameter. I am not doing any dmc anyways so that issue isn't on my radar.

Personally I am doing something with irregular timesteps and have added my own config args for that.

@belerico
Copy link
Member

Beautiful! I'll have it removed asap
Thanks

@belerico belerico linked a pull request Sep 16, 2023 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants