-
Couldn't load subscription status.
- Fork 87
Fix padding_idx=None handling in aten_embedding_bag_padding_idx #2385
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: justinchuby <11205048+justinchuby@users.noreply.github.com>
torch.ops.aten.embedding_bag.padding_idx may see padding_idx=None
❌ 3 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
| # If padding_idx is None, use regular embedding_bag without padding | ||
| if padding_idx is None: | ||
| return aten_embedding_bag( | ||
| weight, indices, offsets, scale_grad_by_freq, mode, sparse, |
Check warning
Code scanning / lintrunner
EDITORCONFIG-CHECKER/editorconfig Warning
| # If padding_idx is None, use regular embedding_bag without padding | ||
| if padding_idx is None: | ||
| return aten_embedding_bag( | ||
| weight, indices, offsets, scale_grad_by_freq, mode, sparse, |
Check warning
Code scanning / lintrunner
RUFF/W291 Warning
See https://docs.astral.sh/ruff/rules/trailing-whitespace
The
aten_embedding_bag_padding_idxfunction was failing when PyTorch passedpadding_idx=None, which is a valid value according to PyTorch's API specification. The function had an assertion that explicitly preventedNonevalues, causing export failures.Changes Made
Updated function signature: Changed
padding_idx: int = -1topadding_idx: Optional[int] = -1to properly reflect PyTorch's API wherepadding_idxcan beNoneRemoved blocking assertion: Removed
assert padding_idx is not Nonethat was preventing validNonevaluesAdded proper None handling: When
padding_idx is None, the function now delegates to the regularaten_embedding_bagfunction, which matches PyTorch's behavior wherepadding_idx=Noneis equivalent to no special padding handlingExample
Before this fix, the following would fail with an AssertionError:
After the fix, it works correctly by falling back to regular embedding_bag behavior when
padding_idx=None.The fix is minimal and surgical, maintaining full backwards compatibility for existing integer values of
padding_idxwhile enabling the previously unsupportedNonecase.Fixes #2145.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.