conv2d/conv3d for LongTensor #20730

ifedan · 2019-05-20T22:39:47Z

Stack:
:black_circle: #20730 conv2d/conv3d for LongTensor 💛

Generates forward conv2d function for LongTensor

Differential Revision: D15423753

Differential Revision: D15423752 Differential Version: 82449506

Differential Revision: D15423753 Differential Version: 82449507

ifedan · 2019-05-20T22:49:15Z

Comments from #20105 #20370
@gchanan:
It also seems like you don't support a larger number of cases, e.g. depthwise, transposed, dilated convolutions.Maybe similar on the AvgPool / SumPool too, but I'm not sure.

gchanan · 2019-05-24T19:54:34Z

test/test_nn.py

    def extra_args(self):
        return self._get_arg('extra_args', False)

+class TestLongTensor(TestCase):


was the comment about depthwise, etc handled?

depthwise is supported only for CUDA. I added transpose2d and dilated2d

Added conv3d, transpose3d, dilated3d

Instead of doing one-off tests like this, how about adding this as an option to the normal NN Module / Criterion tests? There is already some use of this, see check_eval.

@ifedan: Updated

What is the benefit to move them to CriterionTest?

it will check way more combinations of parameters.

The problem with this approach is that initially Tensor is generated as random float and it's impossible to compare it with the long tensor.

Fixed tests

Differential Revision: D15423752 Differential Version: 83368139

Differential Revision: D15423753 Differential Version: 84238345

Differential Revision: D15423753 Differential Version: 84400111

Differential Revision: D15423753 Differential Version: 84408451

gchanan

are you planning to add CUDA support to any of these?

gchanan · 2019-06-18T23:04:17Z

test/test_nn.py

    def extra_args(self):
        return self._get_arg('extra_args', False)

+class TestLongTensor(TestCase):


Instead of doing one-off tests like this, how about adding this as an option to the normal NN Module / Criterion tests? There is already some use of this, see check_eval.

@ifedan: Updated

Differential Revision: D15423753 Differential Version: 85526199

Differential Revision: D15423753 Differential Version: 85526847

ifedan · 2019-06-25T14:11:17Z

@pytorchbot retest this please

Differential Revision: D15423752 Differential Version: 85543831

Differential Revision: D15423753 Differential Version: 85544793

gchanan · 2019-06-25T18:22:02Z

test/test_nn.py

+            test_params['desc'] = 'with_long_tensor' if desc is None else desc + '_with_long_tensor'
+
+        def gen_long_tensor(size):
+            return torch.LongTensor(size=size).random_(-1000, 1000).double().cpu()


can you use torch.randint instead?

Differential Revision: D15423753 Differential Version: 85563780

gchanan

just some small renames, changes.

gchanan · 2019-06-26T17:20:31Z

test/common_nn.py

        constructor_args=(10, 8),
        input_size=(4, 10),
-        reference_fn=lambda i, p: torch.mm(i, p[0].t()) + p[1].view(1, -1).expand(4, 8),
+        reference_fn=lambda i, p, m: torch.mm(i, p[0].t()) + p[1].view(1, -1).expand(4, 8),


the standard in python is to use "_" if you aren't going to use the parameter (throughout this change).

gchanan · 2019-06-26T17:32:06Z

test/common_nn.py

        self.FIXME_no_cuda_gradgrad_comparison = \
            kwargs.get('FIXME_no_cuda_gradgrad_comparison', False)
        self.precision = kwargs.get('precision', 2e-4)
+        self.check_reference_only = kwargs.get('check_reference_only', False)


just reuse the names in CriterionTest -- so this would be check_forward_only.

gchanan · 2019-06-26T17:34:33Z

test/test_nn.py

+            desc = test_params.get('desc', None)
+            test_params['desc'] = 'with_long_tensor' if desc is None else desc + '_with_long_tensor'
+
+        def gen_long_tensor(size):


this name is confusing, because the name implies it returns a long tensor but it actually returns a double tensor.

Also, why do you need to specify CPU?

Discussed offline, going to rename to double_equivalent_of_long_tensor.

gchanan · 2019-06-26T17:35:58Z

test/test_nn.py

+        test_params['input_fn'] = gen_long_tensor_input(test_params['input_size'])
+        test_params['reference_fn'] = reference_fn
+        test_params['check_reference_only'] = True
+        test_params['test_cuda'] = False


comment here.

gchanan · 2019-06-26T17:42:07Z

test/test_nn.py

+        def gen_long_tensor_constructor(constructor):
+            def long_tensor_constructor(*args, **kwargs):
+                cons = constructor(*args, **kwargs)
+                cons._apply(lambda t: Parameter(gen_long_tensor(t.size())) if t.is_floating_point() else t)


I think it's fine to do if t.is_floating_point and isinstance(t, Parameter) or something.

Differential Revision: D15423753 Differential Version: 85666615

ailzhang · 2019-06-26T22:50:06Z

This broke lint. cc @ifedan . [update: xla failure seems to be caused by the master failure. sorry!]~~Also please cc me or @dlibenzi or @asuhan on the thread before landing if this broke xla test.~~ Thanks a lot!

Summary: Pull Request resolved: pytorch/pytorch#20730 Generates forward conv2d function for LongTensor Differential Revision: D15423753 fbshipit-source-id: 0e770b61257cc4c6559581796bf104ef68155c84

facebook-github-bot · 2019-06-26T23:36:18Z

This pull request has been merged in 04fe245.

ifedan added 2 commits May 20, 2019 15:39

V1: Initial commit

ca46f02

Differential Revision: D15423752 Differential Version: 82449506

V1: Initial commit

6b9011c

Differential Revision: D15423753 Differential Version: 82449507

pytorchbot added module: cpu CPU specific problem (e.g., perf, algorithm) module: internals Related to internal abstractions in c10 and ATen module: nn Related to torch.nn module: operators labels May 20, 2019

This was referenced May 20, 2019

Make nn functions configurable for different scalar types #20729

Closed

sum_pool2d/sum_pool3d for LongTensor #20731

Closed

gchanan reviewed May 24, 2019

View reviewed changes

ifedan added 4 commits May 29, 2019 11:23

V2: Changes based on CR:

75d6ee0

Differential Revision: D15423752 Differential Version: 83368139

V3: Merge with parent diff changes

e8f0259

Differential Revision: D15423753 Differential Version: 84238345

V4: (no description)

aaa1055

Differential Revision: D15423753 Differential Version: 84400111

V5: (no description)

1a39c53

Differential Revision: D15423753 Differential Version: 84408451

ifedan changed the title ~~conv2d for LongTensor~~ conv2d/conv3d for LongTensor Jun 6, 2019

gchanan requested changes Jun 18, 2019

View reviewed changes

V7: (no description)

ae98f93

Differential Revision: D15423753 Differential Version: 85526199

ifedan requested a review from gchanan June 25, 2019 02:11

V8: (no description)

45f9b90

Differential Revision: D15423753 Differential Version: 85526847

ifedan added 2 commits June 25, 2019 07:55

V3: (no description)

3918e5b

Differential Revision: D15423752 Differential Version: 85543831

V10: Merge with parent diff changes

fc8c7e9

Differential Revision: D15423753 Differential Version: 85544793

gchanan requested changes Jun 25, 2019

View reviewed changes

V11: (no description)

46f8825

Differential Revision: D15423753 Differential Version: 85563780

ifedan requested a review from gchanan June 25, 2019 18:51

gchanan approved these changes Jun 26, 2019

View reviewed changes

V15: Merge with parent diff changes

bae4d46

Differential Revision: D15423753 Differential Version: 85666615

ifedan changed the base branch from export-D15423752 to master June 26, 2019 21:45

facebook-github-bot closed this in 04fe245 Jun 26, 2019

facebook-github-bot added the merged label Jun 26, 2019

ezyang deleted the export-D15423753 branch July 19, 2019 15:49

mruberry added the Merged label Oct 28, 2020

conv2d/conv3d for LongTensor #20730

conv2d/conv3d for LongTensor #20730

Uh oh!

Conversation

ifedan commented May 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ifedan commented May 20, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gchanan Jun 18, 2019 • edited by ifedan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gchanan left a comment

Choose a reason for hiding this comment

Uh oh!

gchanan Jun 18, 2019 • edited by ifedan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ifedan commented Jun 25, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gchanan left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ailzhang commented Jun 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Jun 26, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ifedan commented May 20, 2019 •

edited

Loading

gchanan Jun 18, 2019 •

edited by ifedan

Loading

gchanan Jun 18, 2019 •

edited by ifedan

Loading

ailzhang commented Jun 26, 2019 •

edited

Loading