-
Notifications
You must be signed in to change notification settings - Fork 85
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: FlatPack environment #188
feat: FlatPack environment #188
Conversation
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
Co-authored-by: Sasha <reallysasha@gmail.com>
…in timestep and state
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We mustn't forget to update the docs as well (mkdocs.yaml)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty much good to go from my side. Happy with the action mask changes! Thanks @RuanJohn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🔥
I just have a few comments regarding doc and types. There needs to be a good reason to go from int32 to float32 when things are inherently integers. What would be the reason here?
Co-authored-by: Clément Bonnet <56230714+clement-bonnet@users.noreply.github.com> Co-authored-by: RuanJohn <33461981+RuanJohn@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Details:
Implements the full
FlatPack
environment with actor-critic and random networks as well as documentation.This environment was previously
Jigsaw
, the core changes fromJigsaw
toFlatPack
is that the blocks (previously pieces) do not have to be placed in exact places on the grid but instead the goal is to fill an empty grid with a given set of blocks. Please see the previous closed PR for comments that have been addressed.Notes:
Training of an a2c agent is complete and can results be viewed at the following link.
closes #187