[BugFix]treat 1‑D MultiDiscrete as MultiCategorical and accept flattened masks#3342
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/3342
Note: Links to docs will display an error until the docs builds have been completed. ❌ 4 New Failures, 1 Cancelled Job, 3 Unrelated FailuresAs of commit c94424b with merge base 9ee9e90 ( NEW FAILURES - The following jobs have failed:
CANCELLED JOB - The following job was cancelled. Please retry:
BROKEN TRUNK - The following jobs failed but were present on the merge base:👉 Rebase onto the `viable/strict` branch to avoid these failures
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
vmoens
left a comment
There was a problem hiding this comment.
✅ Approved — Thank you for this contribution!
@ParamThakkar123 Thanks so much for tackling issue #3242! This is a real pain point for board game environments and similar use cases with 2D action masks.
I took the liberty of making a few small patches on my end — apologies for not waiting for a revision cycle:
-
Simplified
update_maskintensor_specs.py— Removed thetry/exceptblocks andgetattrpattern in favor of direct access (our codebase convention is to fail loudly rather than silently swallow errors) -
Removed blanket exception handling in
gym.py— Same reasoning; added an explanatory comment instead -
Added a test —
test_multidiscrete_action_mask_gymthat reproduces the original bug scenario -
Added documentation — Note in the
ActionMaskdocstring explaining the MultiDiscrete flattening behavior
The core logic of your fix is spot-on. Thanks again for the contribution! 🙏
vmoens
left a comment
There was a problem hiding this comment.
✅ Approved — Thank you for this contribution!
@ParamThakkar123 Thanks so much for tackling issue #3242! This is a real pain point for board game environments and similar use cases with 2D action masks.
I took the liberty of making a few small patches on my end — apologies for not waiting for a revision cycle:
-
Simplified
update_maskintensor_specs.py— Removed thetry/exceptblocks andgetattrpattern in favor of direct access (our codebase convention is to fail loudly rather than silently swallow errors) -
Removed blanket exception handling in
gym.py— Same reasoning; added an explanatory comment instead -
Added a test —
test_multidiscrete_action_mask_gymthat reproduces the original bug scenario -
Added documentation — Note in the
ActionMaskdocstring explaining the MultiDiscrete flattening behavior
The core logic of your fix is spot-on. Thanks again for the contribution! 🙏
|
Thank you so much @vmoens 🫡 |
|
I think it fixes this issue too ? #3197 |
Description
Describe your changes in detail.
Motivation and Context
Why is this change required? What problem does it solve?
If it fixes an open issue, please link to the issue here.
You can use the syntax
close #15213if this solves the issue #15213Fixes #3242
Types of changes
What types of changes does your code introduce? Remove all that do not apply:
Checklist
Go over all the following points, and put an
xin all the boxes that apply.If you are unsure about any of these, don't hesitate to ask. We are here to help!