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: FlatPack environment #188

Merged
merged 91 commits into from
Mar 13, 2024
Merged

feat: FlatPack environment #188

merged 91 commits into from
Mar 13, 2024

Conversation

RuanJohn
Copy link
Contributor

@RuanJohn RuanJohn commented Jul 17, 2023

Details:

Implements the full FlatPack environment with actor-critic and random networks as well as documentation.
This environment was previously Jigsaw, the core changes from Jigsaw to FlatPack 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

RuanJohn and others added 30 commits May 21, 2023 20:04
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>
Copy link
Collaborator

@clement-bonnet clement-bonnet left a 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)

jumanji/environments/packing/flat_pack/reward.py Outdated Show resolved Hide resolved
jumanji/environments/packing/flat_pack/reward.py Outdated Show resolved Hide resolved
sash-a
sash-a previously approved these changes Mar 13, 2024
Copy link
Collaborator

@sash-a sash-a left a 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

clement-bonnet
clement-bonnet previously approved these changes Mar 13, 2024
Copy link
Collaborator

@clement-bonnet clement-bonnet left a 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?

README.md Outdated Show resolved Hide resolved
docs/environments/flat_pack.md Outdated Show resolved Hide resolved
jumanji/environments/packing/flat_pack/env.py Outdated Show resolved Hide resolved
jumanji/environments/packing/flat_pack/env.py Outdated Show resolved Hide resolved
jumanji/environments/packing/flat_pack/generator.py Outdated Show resolved Hide resolved
jumanji/environments/packing/flat_pack/env.py Outdated Show resolved Hide resolved
jumanji/environments/packing/flat_pack/env.py Outdated Show resolved Hide resolved
jumanji/environments/packing/flat_pack/env.py Outdated Show resolved Hide resolved
jumanji/environments/packing/flat_pack/env.py Outdated Show resolved Hide resolved
jumanji/environments/packing/flat_pack/env.py Outdated Show resolved Hide resolved
Co-authored-by: Clément Bonnet <56230714+clement-bonnet@users.noreply.github.com>
Co-authored-by: RuanJohn <33461981+RuanJohn@users.noreply.github.com>
@sash-a sash-a dismissed stale reviews from clement-bonnet and themself via 95389a6 March 13, 2024 16:21
Copy link
Collaborator

@clement-bonnet clement-bonnet left a comment

Choose a reason for hiding this comment

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

LGTM

@clement-bonnet clement-bonnet merged commit bae3ab8 into instadeepai:main Mar 13, 2024
3 checks passed
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.

Feature: Implement FlatPack environment
3 participants