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

added FrozenLake environment #135

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kharyal
Copy link

@kharyal kharyal commented Apr 6, 2021

Added an OpenAI Gym's Frozen Lake like environment.

@Sid-Bhatia-0
Copy link
Member

Hello @kharyal

Thank you for considering contributing to GridWorlds.jl.

Before we can add this environment to this package, we would like to have all the essential components that other environments in this package have. There are several things that need to be done for this:

  1. Generalize this environment. Ideally, we would like to have a randomized procedurally generated environment. The environment grid should not be hard-coded.
  2. Reuse primitives that already existing in this package. For example, there already exist structs for directions Up, Down, Left, Right.
  3. Follow the coding structure, style, and formatting that is consistent with that of other environments. Sorry that this part is not documented as of now. But you can get a fair idea by looking at the code for other environments.
  4. Add tests in test/runtests.jl to make sure everything runs smoothly.
  5. Add a gif in README that demonstrates the rendering of this environment.
  6. Add benchmarks to benchmark.jl and benchmark.md to ensure that the performance is in a range similar to other environments.

Heads up. This is quite a bit of work before this PR becomes ready to be merged 😅 . In case you wish to proceed with doing this, you'll have to get thoroughly familiar with how environments are structured in this package by yourself. I would encourage you to do so, but I would also understand if you do not have enough time and bandwidth for the same. In this case, just inform me in this thread and I'll get this environment added.

@kharyal
Copy link
Author

kharyal commented Apr 8, 2021

Sure, I will do it.

Regards,
Chaitanya

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.

2 participants