Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrong output shape of torch.nn.Conv2d with 2d stride overload #496

Closed
PointerGuide opened this issue Jan 21, 2022 · 6 comments · Fixed by #497
Closed

Wrong output shape of torch.nn.Conv2d with 2d stride overload #496

PointerGuide opened this issue Jan 21, 2022 · 6 comments · Fixed by #497

Comments

@PointerGuide
Copy link

PointerGuide commented Jan 21, 2022

Hi,
according to python's torch documentation (image below),
with W_in: 8, kernelSize: (1, 7), stride: (1, 1), padding: (0, 3) and default dilation (1,1) i should have recived W_out = 8.
I got 2 instead. That could be an issue I think.
image

There isn't that problem with H_out, when i set
W_in: 8, kernelSize: (7, 1), stride: (1, 1), padding: (3, 0)
Screen from
https://pytorch.org/docs/1.9.1/generated/torch.nn.Conv2d.html

@NiklasGustafsson
Copy link
Contributor

I'm seeing the same thing. Thanks for the issue report.

@PointerGuide
Copy link
Author

image
here is the problem
i want to set padding (0,3) but it sets that enum instead

@NiklasGustafsson
Copy link
Contributor

NiklasGustafsson commented Jan 24, 2022

Yes, it's the encoding of padding='same' and padding='valid' that is incorrectly received in the C++ code. Same for Conv3d.

The correct code should look like this:

    torch::nn::Conv2dOptions::padding_t padd =
        (paddingX == -1) && (paddingY == 0) ? torch::kSame :
        (paddingX == 0) && (paddingY == 0) ? torch::kValid :
        (torch::nn::Conv2dOptions::padding_t)torch::ExpandingArray<2>({ paddingX, paddingY });

NiklasGustafsson added a commit to NiklasGustafsson/TorchSharp that referenced this issue Jan 24, 2022
@NiklasGustafsson
Copy link
Contributor

@241721 -- since you already looked into the details yourself, maybe you could do a quick review of PR #497?

NiklasGustafsson added a commit that referenced this issue Jan 24, 2022
@NiklasGustafsson
Copy link
Contributor

@241721 -- thanks for the PR review!

@NiklasGustafsson NiklasGustafsson linked a pull request Jan 24, 2022 that will close this issue
@PointerGuide
Copy link
Author

Thx for your response, i approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants