[tmva][sofie] Implement padding generation for ConvTranspose with explicitly set output_shape

[Sainava]

This Pull request:

Changes or fixes:

• Resolves a TODO in ROperator_ConvTranspose.hxx by dynamically calculating head and tail padding based on the ONNX specification when output_shape is explicitly provided.
• Removes the previous early runtime_error exception and cleans up legacy commented-out padding attempts to streamline the control flow.
• Accurately calculates the effective kernel shape (accounting for dilations) to ensure padding generation remains mathematically correct when dilations > 1.
• Includes a safety check to prevent unsigned integer underflow (size_t) if an invalid or mathematically impossible output_shape is passed to the engine.

Checklist:

• tested changes locally (compiled TMVA locally to verify C++ shape inference logic and syntax)
• updated the docs (if necessary)

[github-actions]

Test Results

    23 files      23 suites   3d 17h 19m 16s ⏱️
 3 860 tests  3 858 ✅ 0 💤 2 ❌
79 542 runs  79 529 ✅ 9 💤 4 ❌

For more details on these failures, see this check.

Results for commit 353271b.

♻️ This comment has been updated with latest results.

[guitargeek]

Thanks for tackling this! The total-padding formula itself is right and matches the ONNX spec, including the output_padding term and the dilated kernel. But there are two correctness issues that need fixing before this can go in, plus a missing test.

1. The padding split is backwards. The spec says:

If (auto_pads == SAME_UPPER): pads[start_i] = total/2;         pads[end_i] = total - total/2
Else:                         pads[start_i] = total - total/2; pads[end_i] = total/2

The reachable path here is always auto_pad == NOTSET (the block above throws for SAME_*), i.e. the Else case — so the larger half belongs at the start, not the end. The commented-out legacy code you removed actually had this part right. (The SAME_UPPER variant is what the PR currently implements unconditionally.)

2. Asymmetric pads are silently discarded downstream, so the result is wrong.

Generate() can't express asymmetric padding: col2im takes a single pad_h/pad_w, so when pads[begin] != pads[end] it prints to std::cout and averages them. The averaged padding no longer matches the fShapeY that ShapeInference() derived, and col2im then walks data_col with the wrong column count, which garbles the whole output rather than just shifting it.

Concretely, for the classic 2× upsample (kernel 3, stride 2, dilation 1, output_shape = 2 * input): total_padding == 1 → pads (0, 1) -> averaged to 0 -> col2im consumes 3 of the 4 input columns. So the feature is correct when total_padding is even and silently wrong when it's odd, and odd is exactly the most common real-world ConvTranspose block. The only signal to the user is a std::cout line at code-generation time.

Either fix is fine by me:

• Minimal: fix the split per the spec, then throw when fAttrPads[i] != fAttrPads[i + fDim] with a message saying asymmetric padding isn't supported yet. Still strictly better than today's blanket throw.
• Proper, and not much more work: generalise col2im in tmva/sofie/src/SOFIE_common_helpers.cxx to take pad_begin/pad_end per axis. Its structure makes this easy — output_h becomes (height + pad_h_begin + pad_h_end - eff_kernel) / stride + 1 and input_row starts at -pad_h_begin; nothing else changes. That would also let the averaging hack in Generate() be deleted.

3. Please add a test.

tmva/sofie/test/generate_input_models.py computes expected outputs with onnx's ReferenceEvaluator, so a make_ConvTranspose2dOutputShape() model (kernel 3, stride 2, output_shape = [2h, 2w]) plus a TEST_INPUTS entry and a TEST(ONNX, ...) in0 TestCustomModelsFromONNX.cxx gets you spec conformance checked automatically, and would have caught both issues above. Since the whole point of the PR is spec conformance, I'd like to see this before merging.

Unrelated: the one red CI check (test-stressgraphics-chrome on fedora44) is a known graphics flake, nothing to do with your change.

[Sainava]

Hi @guitargeek, Thanks for the detailed breakdown! I've pushed an update that attempts to implement the changes you suggested. I've generalized col2im to accept the individual pad_begin and pad_end arguments to support asymmetric padding, corrected the split logic for the NOTSET default, and added the test via make_ConvTranspose2dOutputShape.
Could you please check if this implementation looks alright to you?

[guitargeek]

Thanks for the update! The split direction, the col2im generalization, and the test all look right now. One call site was missed though, and it turns a working configuration into a compile error, so one more round:

1. The fAttrGroup > 1 branch of Generate() still emits the old col2im argument list (ROperator_ConvTranspose.hxx, the if (fDim < 3) block inside the grouped-convolution else branch, around lines 528–541). col2im now takes four pad arguments, so the code generated for any grouped 1d/2d ConvTranspose — including models that work on master today. It no longer compiles. It needs the same treatment you gave the fAttrGroup == 1 branch:

if (fDim == 1)
   out << "1, " << fAttrKernelShape[0] << ",0,0," << fAttrPads[0] << "," << fAttrPads[1]
       << ",1," << fAttrStrides[0] << ",1," << fAttrDilations[0];
else // dim ==2
   out << fAttrKernelShape[0] << "," << fAttrKernelShape[1] << "," << fAttrPads[0] << "," << fAttrPads[2]
       << "," << fAttrPads[1] << "," << fAttrPads[3] << "," << fAttrStrides[0] << "," << fAttrStrides[1] << ","
       << fAttrDilations[0] << "," << fAttrDilations[1];

No existing test uses group > 1 for ConvTranspose, which is why CI is green. Please verify locally that a grouped model still generates compilable code (e.g. python ConvTrans2dModelGenerator.py via TestSofieModels with ngroups=2, or just eyeball the emitted call).

2. Stale comments: the // channels, height, width, kernel_h, kernel_w, pad_h, pad_w, ... comment above both col2im call sites still describes the old signature. Please update them to pad_h_begin, pad_h_end, pad_w_begin, pad_w_end.

3. Test nit (optional but cheap): in make_ConvTranspose2dOutputShape, the explicit pads=[1, 1, 0, 0] happens to be exactly what the code computes from output_shape, so the test can't tell whether SOFIE ignores the attribute (as the spec requires) or uses it. Dropping the pads attribute from the node makes the test strictly stronger at zero cost.

4. Please drop the pure reformatting changes in ROperator_ConvTranspose.hxx (constructor initializer-list reflow, brace spacing on untouched lines). They make the diff harder to review. Run git clang-format so only touched lines are formatted. Even if clang-format in the CI is red, that doesn't matter.

After that this is good to go from my side!

[guitargeek]

LGTM! Thank you very much for the fix and addressing the review comments! I'll just squash the commits, and then the PR can be merged.