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

Reasoning for rendering FPS for dm_control environments #118

Open
realquantumcookie opened this issue Mar 18, 2024 · 1 comment
Open

Reasoning for rendering FPS for dm_control environments #118

realquantumcookie opened this issue Mar 18, 2024 · 1 comment

Comments

@realquantumcookie
Copy link
Contributor

Hello Shimmy Contributors,

I just wanted to ask about the reason behind the metadata['render_fps'] setting for dm_control environments.
In dm_control_compatibility.py#81, it seems like the render_fps is set to the environment's control_timestep converted to millisecond, but I think it is more natural to have it set to int(round((1/control_timestep))? I'm more than happy to submit a PR for this.

@pseudo-rnd-thoughts
Copy link
Member

Looking and thinking about it again.

The dm-control control_timestep docstring says that it is the interval in seconds between actions.
Therefore, 1/control_timesteps correctly computes the frames per second equivalent to the environment's equivalent actions per second.
I'm not sure we want to include the int(round(X)) (1 // X is equivalent as well), maybe, round(X, 3) such that the value is rounded to a reasonable number without losing too much accuracy to the original env.

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

No branches or pull requests

2 participants