cc: @skrah @ezyang
hmmm I guess the commit history is screwed up here but the diff is fine. Let me know if that matters. :P

Looks reasonable I guess? I don't know enough about what annotate does to be able to properly review this. Let me know if you want me to figure out what exactly it's doing.

I think the integration tests are still failing:

May 24 07:47:30 C++ exception with description "max_pool2d_with_indices: internal error: all IntArrayRef sizes must be 2 (max_pool2d_with_indices_out_cuda_template at /var/lib/jenkins/workspace/aten/src/ATen/native/cuda/DilatedMaxPool2d.cu:155)

These are the ones that pass padding.size()==1 and dilation.size()==1. I'm not sure if the JIT is involved here at all.

For reference, this is the sort of call in the tests that leads to padding.size()==1 etc.:

Looking at #22032 I'm starting to think that perhaps the C++ functions in ATen/native should not expect IntArrayRefs of a guaranteed size.

A brief summary of the issue is that

https://github.com/pytorch/examples/blob/1de2ff9338bacaaffa123d03ce53d7522d5dcc2e/cpp/mnist/mnist.cpp#L40

also expects to be able to pass a single integer for kernel_size.

This means that the error checking (and single integer argument expansion) in DilatedMaxPool2d that is currently classified as a workaround could be part of the published C++ API.

In that case the JIT would also not need to special case anything (assuming that is easier for the JIT, which I'm not sure about).

Yea I agree with @ezyang . Basically this PR ensured we do the expansion for users in JIT. To provide the same benefit for our cpp frontend users, I think it's preferred to be in our codegen so that we don't have to handle this in every kernel. I remember I made some progress on that part, I will try to get back to it this week. cc @yf225 to see if he has a better idea here.

Actually I realized codegen is not enough if we want to provide the same convenience to cpp frontend users.
Take max_pool2d(..., IntArrayRef stride={}, IntArrayRef padding={0}, IntArrayRef dilation={1}, ...) as example, we can use codegen to expand padding to {0, 0} and dilation to {1, 1}. (Note to self, this could be done in aten/src/ATen/function_wrapper.py.

1. However this cannot handle stride case as it doesn't have a default case to expand. And the true logic to handle empty init like stride could vary case by case(init as all zeros/ones/...). So this still cannot guarantee we always give initialized stride array to cpp impl.
2. Even we manage to get all the function declaration initialized correctly from simplified representation in native_functions.yaml, this codegen stuff won't work on user end. As cpp frontend user type padding=0, dilation=2, the only way we can handle the expansion is in cpp code.
   Thus I think it's reasonable to say that our aten implementation should take care of these expansions.
3. If we think about adding some logic before invoking this, it's definitely not trivial, as it could vary case by case how to initialize an empty stride. For max_pool it should be the same as kernel_size if empty, what if in other functions it should be initialized as all zeros? In that case it might be simpler to keep this implementation detail in the cpp implementation.

On the other hand, I think it might be reasonable to expect all inputs from JIT frontend to be properly initialized, as JIT is supposed to be statically typed (it doesn't take int into List[int], this should be already handled by BroadcastingListx). So I think this PR that fixes the issue on JIT side still makes sense.
@ezyang Let me know if these makes sense or not :D I will rebase and only fix the JIT part in this PR if it sounds good.
[Update:] On a second thought, if we already enforce our aten implementation should take care of these expansions, is it reasonable for JIT to also take advantage of that? If we allow it, I'm also happy to discard this PR.

As a short term fix, I don't mind just going through and fixing the ATen implementations to do expansion as necessary. (It will be good to write some utility functions first so we aren't doing hella copy pasting).

If we were going to solve this by codegen, I would have imagined some sort of stub function that interposes between Tensor::foo and at::native::foo that does expansions and then passes things along. But I do agree this is a bit involved, and maybe better to just do the simple thing.

And yes, if we do force C++ to handle expansion, we should remove the logic from JIT and PyTorch. The thing that I am concerned about is that we had better do a good job auditing and testing all the places we need to do this, or there are going to be a lot of segfaults in our future.

Thanks @ezyang ! Yea I totally agree some helper functions would be best short term solution. cc @skrah maybe you can set these up in one PR so that they're consistently used in TH porting?
I'm going to close this PR for now, since ATen is guaranteed to handle expansion, JIT doesn't have to repeat the work.

@ailzhang I opened #22277 as a catch-all issue, I can start a PR around mid-July if no one beats me to it.