Farama-Foundation / Gymnasium-Robotics

A collection of robotics simulation environments for reinforcement learning
https://robotics.farama.org/
MIT License
529 stars 85 forks source link

[Bug Report] Setting goal_cell in point_maze causes an error #163

Closed gauthamvasan closed 1 year ago

gauthamvasan commented 1 year ago

Describe the bug Indexing error in point_maze assertion. When I pass the "goal_cell" as a 2D numpy array via the options dict as required here, I get this error:

  File "/home/vasan/RL/lib/python3.10/site-packages/gymnasium_robotics/envs/maze/maze.py", line 294, in reset
    self.maze.maze_map[options["goal_cell"][1], options["goal_cell"][0]]
TypeError: list indices must be integers or slices, not tuple

Solution It's fairly simple. The array indexing code is broken. The assertion should be:

self.maze.maze_map[options["reset_cell"][1]][options["reset_cell"][0]] != 1

System Info

Checklist

Kallinteris-Andreas commented 1 year ago

which environment revision did you use

gauthamvasan commented 1 year ago

PointMaze_UMaze-v3. I used it as follows based on an examples in the docs here

import gymnasium as gym

OPEN_DIVERSE_GR = [
    [1, 1, 1, 1, 1, 1, 1],
    [1, C, C, C, C, C, 1],
    [1, C, C, C, C, C, 1],
    [1, C, C, C, C, C, 1],
    [1, 1, 1, 1, 1, 1, 1]
]

env = gym.make('PointMaze_UMaze-v3', maze_map=OPEN_DIVERSE_GR)
rodrigodelazcano commented 1 year ago

@gauthamvasan thanks for reporting this issue. The problem lies in the assertion as you mentioned. The solution you suggest is correct and should be applied to the following lines https://github.com/Farama-Foundation/Gymnasium-Robotics/blob/ddac411da3907463d8186696b93ef7f72e8892e0/gymnasium_robotics/envs/maze/maze_v4.py#L298 and https://github.com/Farama-Foundation/Gymnasium-Robotics/blob/ddac411da3907463d8186696b93ef7f72e8892e0/gymnasium_robotics/envs/maze/maze_v4.py#L316

@gauthamvasan would you have the time to make a PR? The changes should be applied to maze.py as well as maze_v4.py.

Changes will be included in the next release as well, so after the PR is merged I will suggest working with the source code in the mean time.

gauthamvasan commented 1 year ago

Yes, of course! I'll make a PR shortly. Thanks @rodrigodelazcano

RajGhugare19 commented 1 year ago

I think there is another error with this assertion statement. In the docs describing the pointmaze variation, it is mentioned that:

The data structure to represent the mazes is a list of lists (list[list]) that contains the encoding of the discrete cell positions (i,j) of the maze. Each list inside the main list represents a row i of the maze, while the elements of the row are the intersections with the column index j

This means that "maze_map" is a list of lists. If we provide goal_cell = [i,j] then this should correspond to ith row and the jth column of the "maze_map". But the assertion here checks for the opposite case:

self.maze.maze_map[options["goal_cell"][1]][options["goal_cell"][0]]

Shouldn't the correct assertion be :

self.maze.maze_map[options["goal_cell"][0]][options["goal_cell"][1]] ?

This throws assertion errors whenever we assign valid goals. For example try the following code:

env = gym.make('PointMaze_UMaze-v3') ; env.reset({'goal_cell':np.array([1,2], dtype=np.int32)}

This will throw the following error:

AssertionError: Goal can't be placed in a wall cell, [1 2]

This problem exists in both maze.py and maze_v4.py

rodrigodelazcano commented 1 year ago

Hey @RajGhugare19 I didn't see this comment since the issue was closed, apologize. And thank you for finding this, you are totally right. I just made a PR that fixes this, https://github.com/Farama-Foundation/Gymnasium-Robotics/pull/179.