Skip to content

[6.40] [tmva][sofie] fix double-counted dilation in Conv im2col and adding regression test#22482

Open
root-project-bot wants to merge 1 commit into
root-project:v6-40-00-patchesfrom
root-project-bot:BP_6.40_pull_22474
Open

[6.40] [tmva][sofie] fix double-counted dilation in Conv im2col and adding regression test#22482
root-project-bot wants to merge 1 commit into
root-project:v6-40-00-patchesfrom
root-project-bot:BP_6.40_pull_22474

Conversation

@root-project-bot

Copy link
Copy Markdown

Backport of #22474, requested by @guitargeek. For your information @harz05

Conv generated invalid code and crashed (segfault) for dilation > 1. The operator builds a dilated `_f` weight buffer and expands `fAttrKernelShape` to the effective kernel size, but the im2col call was still passed the real dilation, so dilation was applied twice (expanded kernel + dilation). This produced a negative output dimension and an out-of-bounds write.

Fix: after the dilated `_f` is built, the dense im2col must use dilation 1, since the dilation is already folded into the expanded kernel shape and the `_f` layout. This is a no-op for dilation 1, so existing Conv tests are unaffected.

Also adds a `ConvWithDilation` test (3x3 kernel, dilation 2) with reference output. The suite had no dilation > 1 case before, which is why this was never caught. The full SOFIE test suite passes locally on master.

(cherry picked from commit d42a7a7)
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

Test Results

    22 files      22 suites   3d 9h 32m 4s ⏱️
 3 854 tests  3 854 ✅ 0 💤 0 ❌
76 123 runs  76 123 ✅ 0 💤 0 ❌

Results for commit f55c1d6.

♻️ This comment has been updated with latest results.

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

Labels

clean build Ask CI to do non-incremental build on PR pr:backport

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants