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: implement the sliding tile puzzle env #189

Merged

Conversation

ElshadaiK
Copy link
Contributor

Details:
Implements the full SlidingTilePuzzle environment with actor-critic and random networks as well as documentation.

Notes:
Gifs are still to be updated. Training of an a2c agent is ongoing

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.

This looks really great! Couple minor changes and we got to wait and see how well it performs 🔥

docs/environments/sliding_tile_puzzle.md Outdated Show resolved Hide resolved
jumanji/environments/logic/sliding_tile_puzzle/__init__.py Outdated Show resolved Hide resolved
jumanji/environments/logic/sliding_tile_puzzle/env_test.py Outdated Show resolved Hide resolved
jumanji/environments/logic/sliding_tile_puzzle/env.py Outdated Show resolved Hide resolved
jumanji/environments/logic/sliding_tile_puzzle/env.py Outdated Show resolved Hide resolved
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.

Thank you for the PR! Left a few comments on a first review :)

docs/environments/sliding_tile_puzzle.md Outdated Show resolved Hide resolved
jumanji/environments/logic/sliding_tile_puzzle/env.py Outdated Show resolved Hide resolved
docs/environments/sliding_tile_puzzle.md Outdated Show resolved Hide resolved
docs/environments/sliding_tile_puzzle.md Outdated Show resolved Hide resolved
jumanji/environments/logic/sliding_tile_puzzle/env.py Outdated Show resolved Hide resolved
jumanji/environments/logic/sliding_tile_puzzle/env.py Outdated Show resolved Hide resolved
@carlosgmartin
Copy link

What's the status of this PR? Anything I could help with?

sash-a
sash-a previously approved these changes Jan 11, 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.

Sorry this took so long, but awesome work @ElshadaiK I think it's pretty much ready just some minor comments from my side

jumanji/environments/logic/sliding_tile_puzzle/env.py Outdated Show resolved Hide resolved
jumanji/environments/logic/sliding_tile_puzzle/env.py Outdated Show resolved Hide resolved
jumanji/environments/logic/sliding_tile_puzzle/env.py Outdated Show resolved Hide resolved
jumanji/environments/logic/sliding_tile_puzzle/env.py Outdated Show resolved Hide resolved
jumanji/environments/logic/sliding_tile_puzzle/reward.py Outdated Show resolved Hide resolved
docs/environments/sliding_tile_puzzle.md Outdated Show resolved Hide resolved
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 overall, still investigating why training doesn't work.

@clement-bonnet clement-bonnet force-pushed the feat/add_sliding_tile_puzzle_environment branch from 0284549 to 72af3fe Compare March 12, 2024 14:27
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.

Just some small things we need to add here. Also need to add this mkdocs.yaml and add the gif

docs/environments/sliding_tile_puzzle.md Outdated Show resolved Hide resolved
jumanji/training/configs/config.yaml Outdated Show resolved Hide resolved
jumanji/training/configs/config.yaml Outdated Show resolved Hide resolved
@clement-bonnet clement-bonnet force-pushed the feat/add_sliding_tile_puzzle_environment branch from 5acc203 to 00d6980 Compare March 13, 2024 17:40
clement-bonnet
clement-bonnet previously approved these changes Mar 13, 2024
clement-bonnet
clement-bonnet previously approved these changes Mar 13, 2024
sash-a
sash-a previously approved these changes Mar 13, 2024
@clement-bonnet clement-bonnet dismissed stale reviews from sash-a and themself via b492701 March 13, 2024 20:21
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

@djbyrne djbyrne merged commit a903c4f into instadeepai:main Mar 14, 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.

None yet

5 participants