Skip to content

Validate grad_output in grid_sampler_{2d,3d}_backward - #5584

Merged
torchxpubot merged 2 commits into
mainfrom
harden-grid-sampler-backward-checks
Oct 2, 2026
Merged

torchxpubot merged 2 commits into
mainfrom
harden-grid-sampler-backward-checks

Conversation

@pbielak

@pbielak pbielak commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

XPU was the only backend still skipping the grad_output shape check, so a mismatched grad_output made both backward kernels read out of bounds. CPU, CUDA and MPS already use the hardened helpers added upstream by pytorch/pytorch#191915.

  • 2d: replace check_grid_sampler_common + check_grid_sampler_2d with check_grid_sampler_2d_backward
  • 3d: replace the equivalent block with check_grid_sampler_3d_backward and
    reject bicubic via a standalone TORCH_CHECK_NOT_IMPLEMENTED, mirroring MPS; the torch check is kept until 5-D bicubic lands (Implement bicubic version of grid_sampler_3d #5511)
  • port test_grid_sample_backward_error_checking from upstream test_nn.py

@pbielak
pbielak requested review from dvrogozh and lchen2331 October 1, 2026 15:16
@github-actions github-actions Bot added disable_e2e Disable all e2e test jobs for the PR disable_distributed Disable distributed UT test jobs for the PR labels Oct 1, 2026
Comment thread test/xpu/test_nn_xpu.py Resolved
@pbielak
pbielak requested a review from dvrogozh October 2, 2026 08:30
XPU was the only backend still skipping the `grad_output` shape check, so a
mismatched `grad_output` made both backward kernels read out of bounds. CPU,
CUDA and MPS already use the hardened helpers added upstream by
pytorch/pytorch#191915.

- 2d: switch to `check_grid_sampler_2d_backward`
- 3d: add `check_grid_sampler_backward`; the bicubic-rejecting 3-arg
  `check_grid_sampler_3d` overload is kept until 5-D bicubic lands (#5511)
- port `test_grid_sample_backward_error_checking` from upstream `test_nn.py`

Requested in review of #5511
@pbielak
pbielak force-pushed the harden-grid-sampler-backward-checks branch from 0f0e78a to a8b9dbb Compare October 2, 2026 09:34
@pbielak
pbielak requested a review from tszulist-hbn October 2, 2026 09:37

@dvrogozh dvrogozh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@dvrogozh dvrogozh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The PR now looks good to me per-se, but I see the CI failure which might be related:

op_extended,third_party.torch-xpu-ops.test.xpu.extended.test_ops_xpu.TestCompositeComplianceXPU,test_forward_ad_nn_functional_grid_sample_xpu_float32

Please, help to investigate.

@pbielak

pbielak commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor Author

The PR now looks good to me per-se, but I see the CI failure which might be related:

op_extended,third_party.torch-xpu-ops.test.xpu.extended.test_ops_xpu.TestCompositeComplianceXPU,test_forward_ad_nn_functional_grid_sample_xpu_float32

Please, help to investigate.

Added this test case to the tracking issue: #5452 - all will be fixed with the actual implementation of the Bicubic grid sampler #5511

(This entry also fails in other PR - e.g., https://github.com/intel/torch-xpu-ops/actions/runs/37002662439/job/110852416391?pr=5592 )

@dvrogozh

dvrogozh commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

@pbielak , please, do the similar triage for the recently finished op_ut tests:

op_ut,third_party.torch-xpu-ops.test.xpu.functorch.test_ops_xpu.TestOperatorsXPU,test_vmapjvpall_has_batch_rule_nn_functional_grid_sample_xpu_float32
op_ut,third_party.torch-xpu-ops.test.xpu.functorch.test_ops_xpu.TestOperatorsXPU,test_jvp_nn_functional_grid_sample_xpu_float32
op_ut,third_party.torch-xpu-ops.test.xpu.functorch.test_ops_xpu.TestOperatorsXPU,test_vmapjvpall_nn_functional_grid_sample_xpu_float32
ops.test.xpu.test_ops_fwd_gradients_xpu.TestFwdGradientsXPU,test_forward_mode_AD_nn_functional_grid_sample_xpu_float64
op_ut,third_party.torch-xpu-ops.test.xpu.test_ops_xpu.TestCompositeComplianceXPU,test_forward_ad_nn_functional_grid_sample_xpu_float32

Also, check if other failing tests are a known failures (they don't have grid_sample in their names).

@pbielak

pbielak commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

@pbielak , please, do the similar triage for the recently finished op_ut tests:

op_ut,third_party.torch-xpu-ops.test.xpu.functorch.test_ops_xpu.TestOperatorsXPU,test_vmapjvpall_has_batch_rule_nn_functional_grid_sample_xpu_float32
op_ut,third_party.torch-xpu-ops.test.xpu.functorch.test_ops_xpu.TestOperatorsXPU,test_jvp_nn_functional_grid_sample_xpu_float32
op_ut,third_party.torch-xpu-ops.test.xpu.functorch.test_ops_xpu.TestOperatorsXPU,test_vmapjvpall_nn_functional_grid_sample_xpu_float32
ops.test.xpu.test_ops_fwd_gradients_xpu.TestFwdGradientsXPU,test_forward_mode_AD_nn_functional_grid_sample_xpu_float64
op_ut,third_party.torch-xpu-ops.test.xpu.test_ops_xpu.TestCompositeComplianceXPU,test_forward_ad_nn_functional_grid_sample_xpu_float32

Also, check if other failing tests are a known failures (they don't have grid_sample in their names).

I also checked these and I added them to the tracking issue (same problem of missing bicubic implementation)

@dvrogozh dvrogozh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@dvrogozh

dvrogozh commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

The rest are known failures (same in https://github.com/intel/torch-xpu-ops/actions/runs/36961251529/job/110751670169?pr=5117 for the #5117 which is orthogonal to the PR we review).

@dvrogozh

dvrogozh commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

@torchxpubot merge -f "CI failures are unrelated"

@torchxpubot

Copy link
Copy Markdown
Contributor

Failed to merge PR: Repository rule violations found

A conversation must be resolved before this pull request can be merged.

@dvrogozh

dvrogozh commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

@torchxpubot merge -f "CI failures are unrelated"

@torchxpubot
torchxpubot merged commit 9521dfb into main Oct 2, 2026
54 of 58 checks passed
@torchxpubot

Copy link
Copy Markdown
Contributor

PR has been successfully merged by @dvrogozh (force merge).

@torchxpubot
torchxpubot deleted the harden-grid-sampler-backward-checks branch October 2, 2026 16:54
orrangetabby17 pushed a commit that referenced this pull request Oct 8, 2026
XPU was the only backend still skipping the `grad_output` shape check,
so a mismatched `grad_output` made both backward kernels read out of
bounds. CPU, CUDA and MPS already use the hardened helpers added
upstream by pytorch/pytorch#191915.

- 2d: replace `check_grid_sampler_common` + `check_grid_sampler_2d` with
`check_grid_sampler_2d_backward`
- 3d: replace the equivalent block with `check_grid_sampler_3d_backward`
and
reject bicubic via a standalone `TORCH_CHECK_NOT_IMPLEMENTED`, mirroring
MPS; the torch check is kept until 5-D bicubic lands (#5511)
- port `test_grid_sample_backward_error_checking` from upstream
`test_nn.py`
minmingzhu pushed a commit to minmingzhu/torch-xpu-ops that referenced this pull request Oct 28, 2026
XPU was the only backend still skipping the `grad_output` shape check,
so a mismatched `grad_output` made both backward kernels read out of
bounds. CPU, CUDA and MPS already use the hardened helpers added
upstream by pytorch/pytorch#191915.

- 2d: replace `check_grid_sampler_common` + `check_grid_sampler_2d` with
`check_grid_sampler_2d_backward`
- 3d: replace the equivalent block with `check_grid_sampler_3d_backward`
and
reject bicubic via a standalone `TORCH_CHECK_NOT_IMPLEMENTED`, mirroring
MPS; the torch check is kept until 5-D bicubic lands (intel#5511)
- port `test_grid_sample_backward_error_checking` from upstream
`test_nn.py`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disable_distributed Disable distributed UT test jobs for the PR disable_e2e Disable all e2e test jobs for the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants