Skip to content

fix(interp): use 1/scale_factor coordinate scale for bilinear/bicubic resize - #6851

Open
Lfan-ke wants to merge 1 commit into
Tencent:masterfrom
Lfan-ke:fix/interp-scale-factor
Open

fix(interp): use 1/scale_factor coordinate scale for bilinear/bicubic resize#6851
Lfan-ke wants to merge 1 commit into
Tencent:masterfrom
Lfan-ke:fix/interp-scale-factor

Conversation

@Lfan-ke

@Lfan-ke Lfan-ke commented Jul 26, 2026

Copy link
Copy Markdown

Interp's linear_coeffs/cubic_coeffs always derive the sampling scale from w / outw, but when resizing by a scale factor (no explicit output size) the coordinate scale should be 1 / scale_factor, matching PyTorch. #3555 fixed this for nearest; bilinear/bicubic were missed.

For integer scale factors the two are equal, so this only affects non-integer factors, e.g. F.interpolate(x, scale_factor=1.5, mode='bilinear').

Fix: thread the scale-factor-aware scale (the same expression already used on the nearest path) into linear_coeffs/cubic_coeffs across all backends. The vulkan path drops the resize_type == 2 special-case so bilinear uses the same scale as nearest - the shader already applies the scale generically.

Verification: for a 5-wide input at scale_factor=1.5 (outw=7), bilinear output changes from 0, 0.5714, 1.2857, ... to 0, 0.5, 1.1667, 1.8333, 2.5, 3.1667, 3.8333, matching PyTorch's (dx+0.5)/1.5 - 0.5 mapping exactly. Verified locally on x86 (native) and aarch64 (cross-compiled + qemu) that each backend's output matches the corrected naive reference via test_interp.

Closes #6836

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2e0281602a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/layer/interp.cpp Outdated
float* alpha = (float*)(buf + outw);

linear_coeffs(w, outw, xofs, alpha, align_corner);
linear_coeffs(w, outw, xofs, alpha, align_corner, (output_width || !size_expr.empty()) ? (double)w / outw : 1.0 / width_scale);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve reference-size scaling for dynamic targets

When dynamic_target_size==1 without size_expr, the two-input Interp path gets the target size from reference_blob, but output_width stays 0. This new ternary therefore passes 1.0 / width_scale (usually 1) instead of the old w / outw, so dynamic bilinear/bicubic resizes such as 16→12 sample positions 0..11 rather than spanning the source. Treat dynamic targets like explicit sizes here and in the copied backend calls.

Useful? React with 👍 / 👎.

Comment thread src/layer/vulkan/interp_vulkan.cpp Outdated
Comment on lines +402 to +403
constants[10].f = (output_width || !size_expr.empty()) ? w / (float)outw : 1.f / width_scale;
constants[11].f = (output_height || !size_expr.empty()) ? h / (float)outh : 1.f / height_scale;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Update Vulkan bicubic coefficient scales

This updates the Vulkan nearest/bilinear constants to use 1 / width_scale and 1 / height_scale for scale-factor resizes, but resize_type==3 does not use these constants; the bicubic coefficient pipelines below still receive bottom_blob.w / outw and bottom_blob.h / outh. For non-integer scale-factor bicubic on Vulkan, GPU output remains on the old coordinate grid while the CPU backends now use the corrected scale.

Useful? React with 👍 / 👎.

… resize

Signed-off-by: 林晨 (Leo Cheng) <leo-cheng@vip.qq.com>
@Lfan-ke
Lfan-ke force-pushed the fix/interp-scale-factor branch from 2e02816 to 42cf77b Compare July 26, 2026 14:45
@Lfan-ke

Lfan-ke commented Jul 26, 2026

Copy link
Copy Markdown
Author

Both real, fixed. Added dynamic_target_size to the target-size condition at every call site (naive + all backends, threaded through the interp_bf16s free functions), and gated the Vulkan bicubic constants[2] the same way. Independent check: a dynamic 16->12 resize now spans the source (0.167 ... 14.833, max err 0.0 vs the bug's 0 ... 11); test_interp passes, and scale-factor resizes still use 1/width_scale.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Interp bilinear/bicubic use the wrong sampling scale for non-integer scale_factor (same as #3555, but for bilinear/bicubic)

1 participant