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

Updating bot for compatibility with legacy and Minigrid #121

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Near32
Copy link

@Near32 Near32 commented Mar 14, 2023

  1. Updating bot for it to be compatible with either BabyAI benchmark in its legacy repository, and in its integration within Minigrid...

  2. setup.py is updated in order to:

  • use gym==0.23, which is before introduction of the 'truncated' output, which might be desirable for all, but at least reasonable for operability with all other legacy codebase.
  • use gym-minigrid==1.1, which is before it became Minigrid and new pip does not allow its installation as specified before.

When the mission involves finding a 'grey object', 'wall'-typed objects can be considered, which is not a desired situation.
The previous code was breaking in an assertion error as the returned 'shortest_path_to_obj' would end up being None.
…acy repository, and in its integration within Minigrid.
@saleml
Copy link
Collaborator

saleml commented Mar 15, 2023

Thank you for the PR. I'll review it now.
Note that it has conflicts and cannot be merged as it is now

@saleml
Copy link
Collaborator

saleml commented Mar 15, 2023

Are you sure you have the right versions in the PR ? I get the following error after a fresh install

conda create -n babyai
pip install -e .
cd scripts
python manual_control.py
>>> MiniGridEnv.__init__() missing 1 required positional argument: 'mission_space'

@Roihn
Copy link

Roihn commented Mar 23, 2023

Thanks for your effort! I personally wonder the reason for using the previous minigrid / gym version for bot. Actually after just fixing a few lines of code (np.bool deprecacy, tuple subtraction error, and import issue), we can easily adapt the bot for many babyai tasks.

However, as I further test the bot in all babyai tasks (95 tasks in total till now), just checking whether it can run with no error, I find that in "BabyAI-PutNextS5N2Carrying-v0" (the first episode with seed = 1), it will give "AssertionError: 0nothing left to explore" error.

Actually there are a couple of environments that seem not compatible yet. Here are the list of environments that report errors during a simple test.

BabyAI-PutNextS5N2Carrying-v0
BabyAI-PutNextS6N3Carrying-v0
BabyAI-PutNextS7N4Carrying-v0
BabyAI-KeyInBox-v0
BabyAI-UnlockToUnlock-v0 # Seems to get into a dead loop cuz it takes a really long time until I manually stop the process

What I did for testing is, I make a new environment, create a bot, and take the action as is instructed in each step. I just test every environment with one seed (seed=1) and one episode only, so there may be some hidden errors that still require further testing. Also, I did not check the optimality of the generated demo, which is a huge challenge to test though. Anyway, I hope these existing errors can be somewhat solved if possible (I did not do the testing with your code, so I would be happy to hear if from your side, these errors do not exist).

For compatibility with Minigrid, I wonder how you solve this issue, since each minigrid task does not contain Instruction attribute, which is the key feature for bot to do the planning. (Please correct me if I'm wrong).

Finally, if possible, I wonder whether the bot can support the fully observable scenario, which means the agent no longer needs to explore the environment, and can get the optimal solution after a single glance at the gridworld. It will be quite helpful for my current work if your PR supports this feature. Thanks!

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

4 participants