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.

Sign up for GitHub

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

Jump to bottom

fix(roboticWarehouse): goal conditional #191

Merged
merged 15 commits into from
Aug 19, 2023
Merged

fix(roboticWarehouse): goal conditional #191

merged 15 commits into from
Aug 19, 2023

Conversation

arnupretorius
Copy link
Contributor

Fix the goal conditional for reward in robotic warehouse.

Closes #190

@arnupretorius arnupretorius self-assigned this Jul 19, 2023
@arnupretorius arnupretorius changed the title fix(roboticWarehosue): goal conditional fix(roboticWarehouse): goal conditional Jul 19, 2023
sash-a
sash-a reviewed Jul 24, 2023
View reviewed changes
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 good, just some grammar and some really minor changes

jumanji/environments/routing/robot_warehouse/env.py Outdated Show resolved Hide resolved
jumanji/environments/routing/robot_warehouse/env.py Show resolved Hide resolved
jumanji/environments/routing/robot_warehouse/utils.py Outdated Show resolved Hide resolved
clement-bonnet
clement-bonnet previously approved these changes Jul 30, 2023
View reviewed changes
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.

Looks good to me. Would it be possible to start a few steps of a2c training (with default params) and seed either 1, 2 or 3 to make sure the behavior is fixed?

@arnupretorius
Copy link
Contributor Author

Looks good to me. Would it be possible to start a few steps of a2c training (with default params) and seed either 1, 2 or 3 to make sure the behavior is fixed?

We ran some experiments to confirm, everything works as expected now. In essence, the problem remained the same but with inverted goal positions which caused a mismatch between the rendering. In the runs we did, the agents trained and the rendering looked good. I will also add an updated gif to this PR.

sash-a
sash-a approved these changes Aug 3, 2023
View reviewed changes
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.

LGTM 😄

clement-bonnet
clement-bonnet approved these changes Aug 15, 2023
View reviewed changes
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 fixes! 🚀

RuanJohn
RuanJohn approved these changes Aug 18, 2023
View reviewed changes
Copy link
Contributor

@RuanJohn RuanJohn left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you @arnupretorius 🔥

@clement-bonnet clement-bonnet merged commit 4b843b9 into main Aug 19, 2023
5 checks passed
@clement-bonnet clement-bonnet deleted the 190-fix-rware-reward branch August 19, 2023 21:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sash-a sash-a sash-a approved these changes

@RuanJohn RuanJohn RuanJohn approved these changes

@clement-bonnet clement-bonnet clement-bonnet approved these changes

Assignees

@arnupretorius arnupretorius

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix: robot warehouse rewarding non-goal states
4 participants
@arnupretorius @sash-a @RuanJohn @clement-bonnet