-
Notifications
You must be signed in to change notification settings - Fork 790
[ET-VK][ez] Fix NaN propagation in binary div operations for padded texels #16419
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: gh/SS-JIA/390/base
Are you sure you want to change the base?
Conversation
…exels When the packed dimension size is not a multiple of 4, texture-backed tensors have padding elements in the last texel. For division operations, these padding regions contain 0/0 = NaN, which propagates through subsequent reduction operations and corrupts results. This fix adds conditional padding masking logic to binary_op shaders: - Introduced MASK_PADDING codegen variable to binary_op.yaml - Enabled MASK_PADDING=1 for binary_div and binary_floor_divide ops - Added GLSL preprocessor macro definition in binary_op.glsl - Implemented padding masking logic using modulo arithmetic to correctly identify last texels in batch concatenation scenarios - Padding elements are explicitly zeroed out to prevent NaN propagation The implementation follows GLSL best practices by using Python preprocessing only for macro definition, keeping core shader logic as pure GLSL with standard #ifdef directives. Differential Revision: [D89935220](https://our.internmc.facebook.com/intern/diff/D89935220/) [ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/16419
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Active SEVsThere are 1 currently active SEVs. If your PR is affected, please view them below: ❌ 2 New Failures, 1 Unrelated FailureAs of commit da3a2f1 with merge base 1066e7c ( NEW FAILURES - The following jobs have failed:
UNSTABLE - The following job is marked as unstable, possibly due to flakiness on trunk:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
This PR needs a
|
…or padded texels" When the packed dimension size is not a multiple of 4, texture-backed tensors have padding elements in the last texel. For division operations, these padding regions contain 0/0 = NaN, which propagates through subsequent reduction operations and corrupts results. This fix adds conditional padding masking logic to binary_op shaders: - Introduced MASK_PADDING codegen variable to binary_op.yaml - Enabled MASK_PADDING=1 for binary_div and binary_floor_divide ops - Added GLSL preprocessor macro definition in binary_op.glsl - Implemented padding masking logic using modulo arithmetic to correctly identify last texels in batch concatenation scenarios - Padding elements are explicitly zeroed out to prevent NaN propagation The implementation follows GLSL best practices by using Python preprocessing only for macro definition, keeping core shader logic as pure GLSL with standard #ifdef directives. Differential Revision: [D89935220](https://our.internmc.facebook.com/intern/diff/D89935220/) [ghstack-poisoned]
mergennachin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add this regression test somewhere?
something like this:
x = torch.ones(2, 5, 3, 3)
y = torch.ones(2, 5, 3, 3)
return (x / y).sum()
We should at least add in vulkan test, but also, I wonder if we can expand it to Backend Test harness test that @GregoryComer built
| const int nspill = mod4(out_sizes[packed_dim]); | ||
| const int texels_per_batch = divup4(out_sizes[packed_dim]); | ||
| const bool is_last_texel = (lpos[packed_dim] % texels_per_batch) == (texels_per_batch - 1); | ||
|
|
||
| if (is_last_texel && nspill > 0) { | ||
| // Explicitly set padding elements to 0 to avoid NaN | ||
| [[unroll]] for (int i = nspill; i < 4; i++) { | ||
| out_texel[i] = 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const int texels_per_batch = divup4(out_sizes[packed_dim]); // -> this could be 0, and you're dividing by zero on the next line
i suggest a safer code:
const int nspill = mod4(out_sizes[packed_dim]);
if (nspill > 0) {
const int texels_per_batch = divup4(out_sizes[packed_dim]); // this won't be 0 since we check nspill >0
const bool is_last_texel = (lpos[packed_dim] % texels_per_batch) == (texels_per_batch - 1);
if (is_last_texel) {
[[unroll]] for (int i = nspill; i < 4; i++) {
out_texel[i] = 0;
}
}
}
Stack from ghstack (oldest at bottom):
When the packed dimension size is not a multiple of 4, texture-backed
tensors have padding elements in the last texel. For division operations,
these padding regions contain 0/0 = NaN, which propagates through
subsequent reduction operations and corrupts results.
This fix adds conditional padding masking logic to binary_op shaders:
identify last texels in batch concatenation scenarios
The implementation follows GLSL best practices by using Python
preprocessing only for macro definition, keeping core shader logic
as pure GLSL with standard #ifdef directives.
Differential Revision: D89935220