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

feat: implemented RepeatAction wrapper #990

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Markus28
Copy link
Contributor

@Markus28 Markus28 commented Mar 29, 2024

Description

Implements feature request #652. This still needs a unit test.

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@Markus28 Markus28 changed the title feat: implemented RepeatAction wrapper feat: implemented RepeatAction wrapper Mar 29, 2024
@Kallinteris-Andreas
Copy link
Collaborator

@pseudorndthoughts Do we actually want this feature? In the original issue It was proposed for the use in MuJoCo environments, but MuJoCo already has The frame_skip argument which achieves effectively the same result.

Copy link
Member

@pseudo-rnd-thoughts pseudo-rnd-thoughts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR @Markus28
I just realised that this is very similar to MaxAndSkipObservation (

class MaxAndSkipObservation(
)

@Kallinteris-Andreas It is a simple wrapper that might be helpful for users doing Atari style environment details

If we add this, we will need to add tests and to the website wrapper docs

if num_repeats <= 1:
raise ValueError(
f"Number of action repeats should be greater than 1, but got {num_repeats}"
)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add check that num_repeats is an integer (supporting numpy integers)

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 this pull request may close these issues.

3 participants