Skip to content

Conversation

@jjshoots
Copy link
Contributor

@jjshoots jjshoots commented Apr 13, 2022

Description

This adds a mild domain randomization trait to the Car Racing Env.

Simply pass domain_randomize=True, and the road and background colours will be randomized.

Type of change

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

Before

before1

After

after1
after2
after3

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

@pseudo-rnd-thoughts
Copy link
Contributor

pseudo-rnd-thoughts commented Apr 13, 2022

Could you add a test for hardcore=True?

Edit:
There don't appear to be any tests for lunar lander hardcore=True.
It may make sense to make a test file for all of the box2d, classical control and toy text environment kwargs. Currently, there isn't one

@jjshoots
Copy link
Contributor Author

jjshoots commented Apr 13, 2022

Yea I couldn't find any tests for CarRacing, and I agree with you that there should be. I'll get to implementing one then. Though it remains an interesting question what should be tested. In the lunar lander case, it could be tested because there was already a heuristic algorithm that could solve the environment. There doesn't seem to be one for car racing.

Copy link
Contributor

@RedTachyon RedTachyon left a comment

Choose a reason for hiding this comment

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

Overall looks good, just left one nitpicky comment for code quality

Actually, one thought. Would it make sense to name this something else than hardcore? I know there are some envs that already follow this naming pattern, but it's extraordinarily undescriptive.

Also - we might want to register an extra env to make it easier to create this version, something like "CarRacingHardcore" or whatever Hardcore might get renamed to.

self.grass_color[idx] += 20
else:
# default colours
self.norm_road_color = np.array([0.4, 0.4, 0.4])
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have to store the colors with two different representations? (one is 0-1, the other is 0-255)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, fixed it

### Version History
- v0: Current version
- v1: Current version (0.23.1)
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess v0 can be labelled as the original version; v1 will come live with 0.24.0 at the earliest, so probably better to put that (seeing as 0.23.1 is already out)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Gotcha, fixed it

@jjshoots
Copy link
Contributor Author

@RedTachyon I've changed hardcore to domain_randomize, hopefully that's more descriptive, also registered it.

@jkterry1 jkterry1 merged commit c6deb81 into openai:master Apr 15, 2022
@jjshoots jjshoots deleted the carracing_randomize branch April 15, 2022 15:07
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.

4 participants