Skip to content

[Backend] Fix explicit pooling pads and CUDA TopKV2 largest flag#4627

Open
Fatemanx wants to merge 2 commits into
alibaba:masterfrom
Fatemanx:fix/cuda-yolov10-maxpool-4613
Open

[Backend] Fix explicit pooling pads and CUDA TopKV2 largest flag#4627
Fatemanx wants to merge 2 commits into
alibaba:masterfrom
Fatemanx:fix/cuda-yolov10-maxpool-4613

Conversation

@Fatemanx

Copy link
Copy Markdown
Contributor

Summary

  • Honor CAFFE explicit pads() for non-global pooling across CPU, CUDA, and OpenCL paths.
  • Keep global pooling padding consistent with shape inference by ignoring explicit pads for global pooling.
  • Respect CUDA TopKV2 largest so largest=false returns the smallest values.

Related Issue

Related to #4613

Test Plan

  • cmake --build build_cuda -j$(nproc) --target run_test.out
  • ./build_cuda/run_test.out op/CUDAMaxPoolExplicitPads 0 0 0
  • ./build_cuda/run_test.out op/CUDAMaxPoolExplicitPads 2 0 0
  • ./build_cuda/run_test.out op/CUDATopKV2LargestFlag 0 0 0
  • ./build_cuda/run_test.out op/CUDATopKV2LargestFlag 2 0 0
  • ./build_cuda/run_test.out op/TopKV2 0 0 0
  • ./build_cuda/run_test.out op/TopKV2 2 0 0
  • ./test.sh static

OpenCL execution was not run locally because this environment does not provide an OpenCL executor.

@Qxinyu

Qxinyu commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the fix!

Verified on x86 Linux + CUDA (RTX 3090): both new tests fail on current master, confirming the bugs this PR addresses.
Root causes confirmed:

  1. Pooling: when ONNX explicit pads is present (padType=CAFFE), backends switched padType to VALID without reading pads() values, so pooling ran with zero padding.
  2. CUDA TopKV2: mParams.mDescendFlag = boolDescendFlag ? 1 : -1 tested the param pointer instead of the largest() field, so largest=false was always ignored.

Suggestions on test organization:

  • Both tests are actually backend-agnostic (they exercise CPU/OpenCL as well), so the CUDA prefix in file/test names is misleading.
  • CUDATopKV2LargestFlagTest.cpp: consider merging the largest/smallest cases into the existing test/op/TopKV2Test.cpp (op/TopKV2), which currently only covers the default largest=true path.
  • CUDAMaxPoolExplicitPadsTest.cpp: there is no existing 2D forward pooling test in the repo (only PoolGrad/Pool3D/ROIPooling), so this is effectively the first one — consider renaming it to PoolTest.cpp (e.g. registered as op/MaxPoolExplicitPads) so future 2D pooling cases have a natural home.

Thanks again for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants