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

Bug in BOT Stack for PickupLoc #80

Open
chitwansaharia opened this issue Aug 18, 2019 · 12 comments
Open

Bug in BOT Stack for PickupLoc #80

chitwansaharia opened this issue Aug 18, 2019 · 12 comments
Assignees
Labels
bug Something isn't working

Comments

@chitwansaharia
Copy link
Collaborator

chitwansaharia commented Aug 18, 2019

The BOT stack for BabyAI-PickupLoc-v0 contains a drop subgoal. It is due to the fact that every pickup subgoal automatically adds a drop subgoal. However, for PickupLoc, when the BOT needs to act done after finishing all the subgoals will end up failing the mission, as it would have dropped the object already. @rizar

@maximecb maximecb added the bug Something isn't working label Aug 18, 2019
@rizar
Copy link
Collaborator

rizar commented Aug 19, 2019 via email

@maximecb
Copy link
Collaborator

An important thing to check, after fixing the bug, is that the correctness and performance of the bot is not affected. See the eval_bot script.

@chitwansaharia is this bug blocking you right now?

@chitwansaharia
Copy link
Collaborator Author

Sure. I will try fixing the bug, and check the correctness and performance of the bot afterwards. @maximecb I made a temporary fix to serve my purpose. I removed the drop subgoal manually while generating demos for PickupLoc. So it is not blocking me currently.

@chitwansaharia
Copy link
Collaborator Author

Thanks

Even in the instructions like Pickup the blue ball and go to the red ball, you still need to remove the drop subgoal even if the pickup is not the last thing that needs to be done.
A possible solution I can think of is post-processing stack inserting drop subgoal between every two Pickups (inserting it just before latter Pickup subgoal). Do you think this approach is okay ? @rizar

@akshay-sharma1995
Copy link

Hey @chitwansaharia were you able to fix this bug? I have an older version of Babyai and I am getting Assertion Error assert self.bot.mission.carrying if I try to run the expert for PickupLoc using the learner's actions. This is coming from the Drop Subgoal, which I am guessing because of the issue you mentioned. Before updating to the latest version I just wanted to know whether the bug was fixed or not. If not, can you help me with the workaround you have mentioned about removing the drop subgoal, like when should I be removing it from the subgoals stack.

@maximecb
Copy link
Collaborator

maximecb commented Nov 8, 2019

@chitwansaharia ?

@chitwansaharia
Copy link
Collaborator Author

Hi @akshay-sharma1995 I have not pushed the solution yet. Although, the possible workaround for PickupLoc specifically is just to remove the drop subgoal from the BOT stack. It is just a list of subgoals, and you can easily remove the Drop Subgoal from that list.

Hey @chitwansaharia were you able to fix this bug? I have an older version of Babyai and I am getting Assertion Error assert self.bot.mission.carrying if I try to run the expert for PickupLoc using the learner's actions. This is coming from the Drop Subgoal, which I am guessing because of the issue you mentioned. Before updating to the latest version I just wanted to know whether the bug was fixed or not. If not, can you help me with the workaround you have mentioned about removing the drop subgoal, like when should I be removing it from the subgoals stack.

@chitwansaharia chitwansaharia mentioned this issue Nov 8, 2019
@chitwansaharia
Copy link
Collaborator Author

@rizar Please look at the solution idea I proposed before. If that sounds good, I will go ahead and create the PR. Thanks!

@akshay-sharma1995
Copy link

@chitwansaharia, @maximecb I tried this fix. For pickuploc env I removed the DropSubgoal, but now I am getting an assertion error for the PickupSubGoal assert not self.bot.mission.carrying which means the bot is already carrying something. Just to clarify I am trying to run the Bot along with a trained agent and using the agent's action to update the Bot state.
According to the comment on L554 in the replan function in bot.py
action_taken The last action that the agent took. Can be None, in which case the bot assumes that the action it suggested was taken (or that it is the first iteration).
I should be able to pass the action taken by the trained agent to the Bot and it should be able to replan the path. This setup works for the other two envs i tested (GoToLocal-v0 and GoToObJMaje-v0) but it is giving me the above mentioned error in PickUpLoc-v0 and any other env which has PickupSubgoal. Am I passing the trained agent's action to the Bot in the correct way?

@chitwansaharia
Copy link
Collaborator Author

@akshay-sharma1995, can you post the relevant demo code snippet, so that I can run it and get more idea about the problem you are facing ?

@rizar
Copy link
Collaborator

rizar commented Nov 11, 2019

Sorry for the delay, let me join the discussion.

@chitwansaharia , why do we have to remove DropSubgoal from the stack that correspondsPickup the blue ball and go to the red ball. What harm does DropSubgoal do in this case?

@akshay-sharma1995
Copy link

Hey @chitwansaharia, sorry for the delay. I am trying to run an expert along with a trained learner while evaluation. I made the relevant changes in the babyai/babyai/evaluate.py file. I have attached the code as a pastebin over here: https://pastebin.com/embed_js/FPhi7RWV
I have added made the relevant changes in the ManyEnvs class and batch_evaluate function.
You can run it by using the default evaluation script as mentioned in the babyai docs.
I am able to run it for the GoToLocal-v0 env and the GoToObjMaje-v0 env, but it gives error with the PickupLoc-v0 env. I am assuming there will be same issues with any env with a Pickup subgoal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants