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

Fixed bug in SymbolicObsWrapper #331

Merged
merged 37 commits into from
Mar 14, 2023
Merged

Fixed bug in SymbolicObsWrapper #331

merged 37 commits into from
Mar 14, 2023

Conversation

BolunDai0216
Copy link
Collaborator

@BolunDai0216 BolunDai0216 commented Mar 13, 2023 •
edited
Loading

Description

In the SymbolicObsWrapper, self.grid.grid is indexed as (x_coord, y_coord) while it was indexed in the wrapper as (row, col) which corresponds to (y_coord, x_coord). This PR fixes this discrepancy. Also fixed the wrong reference to miniworld in the inline docs for the wrappers.

Fixes #327

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Checklist:

  • I have run the pre-commit checks with pre-commit run --all-files (see CONTRIBUTING.md instructions to set it up)
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

BolunDai0216 and others added 30 commits January 22, 2023 22:31
@BolunDai0216
added docstrings to babyai goto
d5d8b72
@BolunDai0216
added placeholder docstrings to all babyai envs
7467e0a
@BolunDai0216
updated babyai registration
f3c6acd
@BolunDai0216
updated babyai docstrings for open, other, pickup, and putnext
ca7c4ca
@BolunDai0216
updated docstrings for unlock
acfdbdb
@BolunDai0216
added docstrings for synth
adfe31b
@BolunDai0216
updated docstrings for synth
bbd6504
@BolunDai0216
updated docstring
d1b8139
@BolunDai0216
updated test
6e1d3b2
@BolunDai0216
added documentation to some of the wrappers
bd8b313
@BolunDai0216
added documentation for the wrappers
6acfbb4
@BolunDai0216
passed pre-commit
fd6285e
@BolunDai0216
updated isort to 5.12.0
c1e82c7
@BolunDai0216
removed set_trace
798b2fb
@BolunDai0216
fixed generate website test error
6a141af
@BolunDai0216
fixed generate website test error for non-str entry points
86f9298
@BolunDai0216
fixed generate website test error for non-str entry points in gen_env…
2444308 
…s_display
@BolunDai0216
Merge branch 'Farama-Foundation:master' into master
3c9271a
@BolunDai0216
fixed bug in reset() of DirectionObsWrapper and added documentation
5e90c82
@BolunDai0216
deleted unused import pdb
a24b578
@BolunDai0216
renamed StateBonus to PositionBonus
6beb137
@BolunDai0216
renamed state bonus test to position bonus
2e8f4a3
@BolunDai0216
added tests to directionobs and symbolicobs
abb1281
@BolunDai0216
updated doc for PositionBonus
912afc4
@pseudo-rnd-thoughts
Update wrappers.py
7357521
@BolunDai0216
Merge branch 'Farama-Foundation:master' into master
bf3ae7a
@BolunDai0216
ran pre-commit
38b552e
@BolunDai0216
removed used actions
7e30a98
@BolunDai0216
Merge branch 'Farama-Foundation:master' into master
2ecd9b6
@BolunDai0216
Merge branch 'master' of https://github.com/BolunDai0216/Minigrid
66ca7e5
pseudo-rnd-thoughts
pseudo-rnd-thoughts requested changes Mar 13, 2023
View reviewed changes
Copy link
Member

@pseudo-rnd-thoughts pseudo-rnd-thoughts left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, could you add a short test to show that this works

Also, you might want to use branches such that there are not so many commits unrelated to the PR.

@BolunDai0216 BolunDai0216 mentioned this pull request Mar 14, 2023
Closed
@pseudo-rnd-thoughts pseudo-rnd-thoughts merged commit d537e1c into Farama-Foundation:master Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pseudo-rnd-thoughts pseudo-rnd-thoughts pseudo-rnd-thoughts approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Question] Is SymbolicObsWrapper wrong in Dist Shift?
2 participants
@BolunDai0216 @pseudo-rnd-thoughts