Move ConstantPadNd into ATen #10885

wdhorton · 2018-08-26T20:08:14Z

Addresses #9499. Completed work on the forward function, tests should be passing for that. Working on backward function now.

vishwakftw

I didn't check the math.

aten/src/ATen/native/ConstantPadNd.cpp

+    std::vector<int64_t> new_shape;
+
+    for (int i = 0; i < l_diff; i ++) {
+        new_shape.push_back(input_sizes[i]);


aten/src/ATen/native/ConstantPadNd.cpp

+        auto pad_idx = pad.size() - ((i + 1) * 2);
+        auto new_dim = input_sizes[l_diff + i] + pad[pad_idx] + pad[pad_idx + 1];
+        AT_CHECK(new_dim > 0, "input is too small");
+        new_shape.push_back(new_dim);


aten/src/ATen/native/ConstantPadNd.cpp

+    auto output = at::empty(new_shape, input.options());
+    output.fill_(value);
+
+    auto c_input = input;


aten/src/ATen/native/ConstantPadNd.cpp

+    for (int i = l_diff; i < l_inp; i++) {
+        auto pad_idx = pad.size() - (i - l_diff + 1) * 2;
+        if (pad[pad_idx] < 0) {
+            c_input = c_input.narrow(i, -pad[pad_idx], c_input.sizes()[i] + pad[pad_idx]);


aten/src/ATen/native/ConstantPadNd.cpp

+            c_input = c_input.narrow(i, -pad[pad_idx], c_input.sizes()[i] + pad[pad_idx]);
+        }
+        if (pad[pad_idx + 1] < 0) {
+            c_input = c_input.narrow(i, 0, c_input.sizes()[i] + pad[pad_idx + 1]);


aten/src/ATen/native/ConstantPadNd.cpp

+    for (int i = l_diff; i < l_inp; i++) {
+        auto pad_idx = pad.size() - (i - l_diff + 1) * 2;
+        if (pad[pad_idx] > 0) {
+            c_output = c_output.narrow(i, pad[pad_idx], c_output.sizes()[i] - pad[pad_idx]);


aten/src/ATen/native/ConstantPadNd.cpp

+            c_output = c_output.narrow(i, pad[pad_idx], c_output.sizes()[i] - pad[pad_idx]);
+        }
+        if (pad[pad_idx + 1] > 0) {
+            c_output = c_output.narrow(i, 0, c_output.sizes()[i] - pad[pad_idx + 1]);


aten/src/ATen/native/ConstantPadNd.cpp

+            c_output = c_output.narrow(i, 0, c_output.sizes()[i] - pad[pad_idx + 1]);
+        }
+    }
+    c_output.copy_(c_input);


wdhorton · 2018-08-27T23:12:55Z

Finished the implementation of the backward function, so I took off the [WIP] on the PR title. Should be ready to review.

wdhorton · 2018-08-28T00:25:37Z

Getting this error in the failing builds:

00:17:15 ======================================================================
00:17:15 ERROR: test_literal (__main__.TestScript)
00:17:15 ----------------------------------------------------------------------
00:17:15 Traceback (most recent call last):
00:17:15   File "test_jit.py", line 2341, in test_literal
00:17:15     self.checkScript(func3, (a.item(), b.item()), optimize=True)
00:17:15   File "test_jit.py", line 2019, in checkScript
00:17:15     self.checkScript(source, inputs, optimize, outputs, script.__name__, capture_output, frames_up=2)
00:17:15   File "test_jit.py", line 2029, in checkScript
00:17:15     outputs_ge = ge(*inputs)
00:17:15 RuntimeError: Unsupported value kind: Tuple

I don't know how my changes could've impacted the JIT.

Justin4411 · 2018-08-28T00:41:11Z

how can I make these as clear as possible to read? On Monday, August 27, 2018 7:13 PM, William Horton <notifications@github.com> wrote: Finished the implementation of the backward function, so I took off the [WIP] on the PR title. Should be ready to review.— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread. | | Virus-free. www.avg.com |

Justin4411 · 2018-08-28T00:42:42Z

how can I make this as clear as possible to read? without all the comments and coeds

vishwakftw · 2018-08-28T06:37:39Z

@pytorchbot retest this please

vishwakftw · 2018-08-28T06:38:20Z

@wdhorton #10923 has sent in a patch for this bug I think, which is why I have restarted the tests.

ssnl

Thanks for the patch! Sorry for the late review. The port looks great. I think we can improve upon the original code a bit and commented inline. Let me know if you have any questions!

tools/autograd/templates/Functions.cpp

+    auto l_pad = pad.size() / 2;
+    auto l_diff = l_inp - l_pad;
+
+    auto grad_input = Variable(at::zeros(self.sizes(), grad.options()));


aten/src/ATen/native/ConstantPadNd.cpp

+
+    auto l_pad = pad.size() / 2;
+    auto l_diff = l_inp - l_pad;
+    AT_CHECK(l_inp >= l_pad, "Padding length too large");


aten/src/ATen/native/ConstantPadNd.cpp

+    for (int i = 0; i < l_pad; i++) {
+        auto pad_idx = pad.size() - ((i + 1) * 2);
+        auto new_dim = input_sizes[l_diff + i] + pad[pad_idx] + pad[pad_idx + 1];
+        AT_CHECK(new_dim > 0, "input is too small");


tools/autograd/templates/Functions.cpp

+            cg_output = cg_output.narrow(i, 0, cg_output.size(i) - pad[pad_idx + 1]);
+        }
+    }
+    cg_input.copy_(cg_output);


facebook-github-bot

SsnL has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

weiyangfb · 2018-09-11T20:29:30Z

@wdhorton Thanks for a great work! Please address comments when catch a chance!

wdhorton · 2018-09-27T13:18:50Z

@weiyangfb @ssnl could you take another look at this?

tools/autograd/templates/Functions.cpp

+
+    auto cg_input = grad_input;
+    for (int i = l_diff; i < l_inp; i++) {
+        auto pad_idx = pad.size() - (i - l_diff + 1) * 2;


tools/autograd/templates/Functions.cpp

+
+    auto cg_output = grad;
+    for (int i = l_diff; i < l_inp; i++) {
+        auto pad_idx = pad.size() - (i - l_diff + 1) * 2;


facebook-github-bot

SsnL has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot

SsnL is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Summary: Addresses #9499. Completed work on the forward function, tests should be passing for that. Working on backward function now. Pull Request resolved: pytorch/pytorch#10885 Differential Revision: D9643786 Pulled By: SsnL fbshipit-source-id: 2930d6f3d2975c45b2ba7042c55773cbdc8fa3ac

wdhorton requested review from apaszke, colesbury, ezyang, gchanan, soumith, zdevito, anderspapitto, bddppq, dzhulgakov, houseroad, jamesr66a, smessmer and Yangqing as code owners August 26, 2018 20:08

vishwakftw reviewed Aug 27, 2018

View reviewed changes

wdhorton changed the title ~~[WIP] Move ConstantPadNd into ATen~~ Move ConstantPadNd into ATen Aug 27, 2018

wdhorton force-pushed the constant-pad-native branch from a19ce9f to 46c1a32 Compare August 27, 2018 23:55

wdhorton mentioned this pull request Aug 30, 2018

Move ConstantPadNd into ATen #9499

Closed

ssnl reviewed Sep 4, 2018

View reviewed changes

facebook-github-bot reviewed Sep 4, 2018

View reviewed changes

weiyangfb added the awaiting changes label Sep 11, 2018

vishwakftw reviewed Sep 28, 2018

View reviewed changes

tools/autograd/templates/Functions.cpp Outdated

auto cg_input = grad_input;

for (int i = l_diff; i < l_inp; i++) {

auto pad_idx = pad.size() - (i - l_diff + 1) * 2;

This comment was marked as off-topic.

Sign in to view

vishwakftw reviewed Sep 28, 2018

View reviewed changes

tools/autograd/templates/Functions.cpp Outdated

auto cg_output = grad;

for (int i = l_diff; i < l_inp; i++) {

auto pad_idx = pad.size() - (i - l_diff + 1) * 2;

This comment was marked as off-topic.

Sign in to view

facebook-github-bot reviewed Oct 25, 2018

View reviewed changes

wdhorton added 19 commits October 26, 2018 01:27

Forward function for ConstantPadNd

10a60b9

Add onnx definition

9a132d4

Fix onnx definition

976d1e8

Fix onnx signature

8168691

Rename to self

869d2f2

Backward function.

d2e577c

remove import

38e841d

Use .size(i)

c8340bd

Use emplace_back

efa3604

Change value to f for onnx

c6272c5

Remove Variable wrapper

1dabe46

Make error message longer

d5f4832

Add check that pad length is even, and improve error message

ee7d419

Update with optimizations.

c2473dd

Add value to constant_pad_nd call in backward.

1fbe788

semicolons

dcee49c

fix derivatives.yaml

31ee9e3

Add to aten_interned_strings.h

4c1a4c7

Use at:: not at::native in backward.

f2109c4

ssnl force-pushed the constant-pad-native branch from b4d82f2 to f2109c4 Compare October 26, 2018 05:28

ssnl closed this Oct 26, 2018

ssnl reopened this Oct 26, 2018

facebook-github-bot reviewed Oct 26, 2018

View reviewed changes

facebook-github-bot closed this in 1bec8f7 Oct 26, 2018

ezyang added open source merged labels Jun 24, 2019

Move ConstantPadNd into ATen #10885

Move ConstantPadNd into ATen #10885

Conversation

wdhorton commented Aug 26, 2018 • edited

vishwakftw left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

wdhorton commented Aug 27, 2018

wdhorton commented Aug 28, 2018

Justin4411 commented Aug 28, 2018 via email

Justin4411 commented Aug 28, 2018

vishwakftw commented Aug 28, 2018

vishwakftw commented Aug 28, 2018

ssnl left a comment

Choose a reason for hiding this comment

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

facebook-github-bot left a comment

Choose a reason for hiding this comment

weiyangfb commented Sep 11, 2018

wdhorton commented Sep 27, 2018

This comment was marked as off-topic.

This comment was marked as off-topic.

facebook-github-bot left a comment

Choose a reason for hiding this comment

facebook-github-bot left a comment

Choose a reason for hiding this comment

wdhorton commented Aug 26, 2018 •

edited