🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/154279

• 📄 Preview Python docs built from this PR
• 📄 Preview C++ docs built from this PR
• ❓ Need help or want to give feedback on the CI? Visit the bot commands wiki

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Attention! native_functions.yaml was changed

If you are adding a new function or defaulted argument to native_functions.yaml, you cannot use it from pre-existing Python frontend code until our FC window passes (two weeks). Split your PR into two PRs, one which adds the new C++ functionality, and one that makes use of it from Python, and land them two weeks apart. See https://github.com/pytorch/pytorch/wiki/PyTorch's-Python-Frontend-Backward-and-Forward-Compatibility-Policy#forwards-compatibility-fc for more info.

Caused by:

• aten/src/ATen/native/native_functions.yaml

@pytorchbot rebase

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

Successfully rebased feat/conv-transpose-pad-same-cpp onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout feat/conv-transpose-pad-same-cpp && git pull --rebase)

Could someone please trigger the CI?
Soft ping to: @Skylion007 @albanD @shoumikhin. Thank you in advance!

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@pytorchbot rebase

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

Successfully rebased feat/conv-transpose-pad-same-cpp onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout feat/conv-transpose-pad-same-cpp && git pull --rebase)

Hi @Skylion007, could you please take another look at this PR when you have a moment? Also, if possible, remove the Stale label.

@pytorchbot rebase

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

Successfully rebased feat/conv-transpose-pad-same-cpp onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout feat/conv-transpose-pad-same-cpp && git pull --rebase)

Can somebody remove the Stale label and trigger the CI?

Also, if there are any changes needed in this PR, please let me know.

Soft ping to: @Skylion007, @albanD, @shoumikhin and @colesbury. Thank you in advance!

Thanks for removing the stale label and triggering the CI.

The build on the failing job was killed with SIGKILL

sccache: Compiler killed by signal 9

I think this is unrelated to the changes in this PR.

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Merge failed

Reason: 2 jobs have failed, first few of them are: trunk / linux-jammy-cuda12.8-py3.10-gcc11 / test (default, 5, 5, linux.g6.4xlarge.experimental.nvidia.gpu), trunk / linux-jammy-cuda12.8-py3.10-gcc11 / test (default, 3, 5, linux.g6.4xlarge.experimental.nvidia.gpu)

I was investigating the errors, and I can at least understand why.

Traceback

It originates from the changes in common_method_invocations.py

Where for the failing test, as the OpInfo contains this ref (doesn't exist for traditional convolution)

    OpInfo('nn.functional.conv_transpose1d',
           # `ref` for this function is backward of
           # corresponding `conv*d`
           ref=partial(conv_transpose_ref, fn=torch.nn.functional.conv_transpose1d),

The function defined in the ref calls for these gradient functions

    grad_fn_map = {torch.nn.functional.conv_transpose1d: torch.nn.grad.conv1d_input,
                   torch.nn.functional.conv_transpose2d: torch.nn.grad.conv2d_input,
                   torch.nn.functional.conv_transpose3d: torch.nn.grad.conv3d_input}

Which calls for convolution_backward, which doesn't support padding as a string.

    return torch.ops.aten.convolution_backward(
        grad_output,
        input,
        weight,
        None,
        _single(stride),
        _single(padding),
        _single(dilation),
        False,
        [0],
        groups,
        (True, False, False),
    )[0]

Why it doesn't happen for normal convolution?

It doesn't happen for normal convolution because they don't have this ref

    OpInfo('nn.functional.conv1d',
           aliases=('conv1d',),
           aten_name='conv1d',
           dtypes=floating_and_complex_types_and(torch.int64, torch.float16, torch.bfloat16),
           dtypesIfCUDA=floating_and_complex_types_and(torch.float16, torch.chalf,

The conv{1,2,3}d_input functions defined in torch/nn/grad.py doesn't use padding as a string with regular convolution in the codebase.

Next Steps

I see 3 main options:

1. Conditionally skip this test and similar ones.
   • I couldn't find in the class OpInfo a way to skip a test based on the function arguments, so I probably would have to skip in the test body.
2. Remove the changes in common_methods_invocations to don't run this test.
3. Create a new overload for convolution_backward to support padding as a string.
   • I don't know how feasible this is for me, because it seems this is specific for GPU, and I don't have easy access to one.

cc @albanD @jbschlosser

Ho that is very interesting!
Thanks for the details on the investigation!

Let's double check with @jbschlosser if 1) is feasible.
Otherwise 2 is ok.

Skipping / xfailing a particular test only for a given set of samples (AKA option 1 above) is possible, albeit somewhat involved. I think an option 4 could be to update the transposed convolution ref to handle string padding, no?

I think an option 4 could be to update the transposed convolution ref to handle string padding, no?

I will look into it. I still haven't looked what convolution_backward does, but if it's possible to simply do what I did in the C++ definitely is possible!

What I am thinking in doing is the following in the conv_transpose_ref function:

• If it's "valid": replace padding with 0
• If it's "same":
  1. compute the minimum padding;
  2. replace the padding argument with the computed one;
  3. adjust the out value if it's asymmetric.

Is this a valid solution, or do I need to do something else?

Hi @jbschlosser, Can you check the latest commit (c113a2d) and verify if it's what you had in mind?

@pytorchbot rebase

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

Successfully rebased feat/conv-transpose-pad-same-cpp onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout feat/conv-transpose-pad-same-cpp && git pull --rebase)

@pytorchbot rebase

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

Successfully rebased feat/conv-transpose-pad-same-cpp onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout feat/conv-transpose-pad-same-cpp && git pull --rebase)