babyai icon indicating copy to clipboard operation
babyai copied to clipboard

Bug in BOT Stack for PickupLoc

Open chitwansaharia opened this issue 5 years ago • 12 comments

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

chitwansaharia avatar Aug 18 '19 21:08 chitwansaharia

Thanks does sound like a bug. I guess the bot should behave differently when it needs to pick smth up in the end of the execution and not in the middle.

Would you like to try fixing it?

On Sun, 18 Aug 2019 at 17:25, Maxime Chevalier-Boisvert < [email protected]> wrote:

Assigned #80 https://github.com/mila-iqia/babyai/issues/80 to @rizar https://github.com/rizar.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/mila-iqia/babyai/issues/80?email_source=notifications&email_token=AAE7YYS2WHS54PSF6TMU5LTQFG44ZA5CNFSM4IMUDUCKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOTDO527Q#event-2564676990, or mute the thread https://github.com/notifications/unsubscribe-auth/AAE7YYWF6H542V3IUVRAQ5TQFG44ZANCNFSM4IMUDUCA .

rizar avatar Aug 19 '19 17:08 rizar

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?

maximecb avatar Aug 20 '19 15:08 maximecb

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 avatar Aug 20 '19 15:08 chitwansaharia

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

chitwansaharia avatar Aug 22 '19 18:08 chitwansaharia

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.

akshay-sharma1995 avatar Nov 07 '19 18:11 akshay-sharma1995

@chitwansaharia ?

maximecb avatar Nov 08 '19 17:11 maximecb

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 avatar Nov 08 '19 21:11 chitwansaharia

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

chitwansaharia avatar Nov 08 '19 21:11 chitwansaharia

@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?

akshay-sharma1995 avatar Nov 09 '19 22:11 akshay-sharma1995

@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 ?

chitwansaharia avatar Nov 10 '19 00:11 chitwansaharia

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?

rizar avatar Nov 11 '19 14:11 rizar

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.

akshay-sharma1995 avatar Nov 11 '19 22:11 akshay-sharma1995