Allow support for negative dimension argument for all functions #1108

albanD · 2017-03-26T15:58:19Z

Add WrapDim plugin to cwrap that allows to support negative dimension index.
Add generic test for negative dimension.
Add support for negative dimension for all functions that can take a dimension as parameter.

apaszke

Looks good, but we need to add tests for all those functions in CUDA and especially in autograd. Maybe it'd be possible to reuse the same code somehow (Variables have a very similar API).

test/test_autograd.py

@@ -1059,6 +1059,7 @@ def prod_single_zero(dim_size):
    (Scatter, (0,), ((M, S), gather_variable((S, S), 1, M), (S, S))),
    (Scatter, (1,), ((M, S), gather_variable((M, S // 2), 0, S), (M, S // 2)), 'dim1'),
    (Concat, (0,), ((1, S, S), (2, S, S), (3, S, S))),
+    (Concat, (-1,), ((S, S, 1), (S, S, 2), (S, S, 3)), 'negdim'),


test/test_torch.py

+# 4th argument is the supported types of call:
+# 0: method called as tensor.name(...)
+# 1: method called as tensor.name_(...)
+# 2: method called as torch.name(tensor, ...)


test/test_torch.py

+            x = torch.randn(*tensor_arg)
+            ndim = len(tensor_arg)
+
+        n_dim_to_test = 0


test/test_torch.py

+
+        for dims_val in combinations(range(ndim), n_dim_to_test):
+            arg = arg_constr()
+            arg_neg = copy.copy(arg)


test/test_torch.py

+                b = getattr(torch, name)(x, *arg_neg)
+                self.assertEqual(a, b)
+
+    return tmp


test/test_torch.py

+
+
+def idx_tensor(size, max_val):
+    return torch.LongTensor(*size).random_() % max_val


test/test_torch.py

+            x = []
+            for arg in tensor_arg:
+                x.append(torch.randn(*arg))
+                ndim = len(arg)


tools/cwrap/cwrap.py

@@ -33,7 +33,8 @@ class cwrap(object):
    FUNCTION_CALL_TEMPLATE = Template("$capture_result$cname($call_arg);")

    DEFAULT_PLUGIN_CLASSES = [ArgcountChecker, ConstantArguments, OptionalArguments,
-                              ArgumentReferences, BeforeAfterCall, ReturnArguments, GILRelease]
+                              ArgumentReferences, BeforeAfterCall, ReturnArguments,
+                              GILRelease, WrapDim]


tools/cwrap/plugins/WrapDim.py

+class WrapDim(CWrapPlugin):
+
+    DIM_WRAP_TEMPLATE = Template(
+        """if (${arg_to_wrap} < 0) ${arg_to_wrap} += ${arg_tensor}->nDimension;""")


tools/cwrap/plugins/WrapDim.py

+            arg_tensor = arg.get('wrap_dim')
+
+            new_code.append(self.DIM_WRAP_TEMPLATE.substitute(
+                arg_to_wrap="arg_" + arg.get('formal_name', arg['name']),


torch/autograd/_functions/reduce.py

-
-                grad_idx_tuple = idx_tuple[:self.dim] + (zero_idx,) + idx_tuple[self.dim + 1:]
-                grad_input[grad_idx_tuple] = grad_output[idx_tuple] * input_copy.prod()
+            if len(single_zero_idx) > 0:


test/test_autograd.py

+            new_constructor_args[arg_idx] *= dim_perm[i]
+            if dim_perm[i] == -1:
+                test_name += "_negdimarg" + str(arg_idx)
+        new_constructor_args = tuple(new_constructor_args)


test/test_autograd.py

+            new_args[arg_idx] *= dim_perm[i]
+            if dim_perm[i] == -1:
+                test_name += "_negdimarg" + str(arg_idx)
+        new_args = tuple(new_args)


test/test_torch.py

+def make_neg_dim_test(name, tensor_arg, arg_constr, types, extra_dim=0):
+    def neg_dim_test(self):
+        if isinstance(tensor_arg, list):
+            assert 0 not in types and 1 not in types


test/test_torch.py

+for decl in neg_dim_tests:
+    if len(decl) == 4:
+        name, tensor_arg, arg_constr, types = decl
+        extra_dim = 0


tools/cwrap/plugins/WrapDim.py

+        """${arg_tensor}->nDimension""")
+
+    DIM_WRAP_CHECK_TEMPLATE = Template(
+        """THPUtils_assert(${arg_to_wrap} >= -(${ndim}) && ${arg_to_wrap} < (${ndim}),


tools/cwrap/plugins/WrapDim.py

+         "dimension out of range (expected to be in range of [%d, %d], but got %d)",
+         -(${ndim}), (${ndim})-1, ${arg_to_wrap})""")
+
+    DIM_WRAP_CODE_TEMPLATE = Template(


tools/cwrap/plugins/WrapDim.py

+            arg_tensor = params[0]
+
+            arg_tensor = "arg_" + arg_tensor
+            arg_to_wrap = "arg_" + arg.get('assign_name', arg['name'])


torch/csrc/generic/methods/Tensor.cwrap

@@ -191,6 +191,8 @@ static PyObject * THPTensor_(select)(THPTensor *self, PyObject *args)
    return NULL;

  int ndim = THTensor_(nDimension)(LIBRARY_STATE self->cdata);
+  if (dim<0) dim += ndim;


tools/cwrap/plugins/WrapDim.py

+            if 'wrap_dim' not in arg:
+                continue
+
+            params = arg.get('wrap_dim').split("+")


soumith · 2017-04-06T23:53:51Z

Thanks a lot Alban!

unswitch predicate equality fix

pytorch-bot · 2022-09-01T18:52:14Z

🔗 Helpful Links

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

📄 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 our office hours

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.

Co-authored-by: Sukru Eryilmaz <seryilmaz@computelab-dgx1v-32.nvidia.com>

apaszke reviewed Mar 26, 2017

View reviewed changes

albanD force-pushed the cwrap_wrap_dim branch from 3e68f2e to 3d30e38 Compare March 28, 2017 15:52

apaszke reviewed Apr 2, 2017

View reviewed changes

apaszke approved these changes Apr 6, 2017

View reviewed changes

soumith force-pushed the cwrap_wrap_dim branch from 6fd3f3b to 928afc0 Compare April 6, 2017 23:28

Add TH_INDEX_BASE to nDimension and stride functions

ae1c365

soumith force-pushed the cwrap_wrap_dim branch 3 times, most recently from 5013762 to 448bf55 Compare April 6, 2017 23:34

albanD and others added 2 commits April 6, 2017 16:37

Allow support for negative dimension argument for all functions

f0c7124

Merge commit 'ae1c365dbdbf667ae24c57eec9f2e6b9debf16bd'

37d9568

soumith force-pushed the cwrap_wrap_dim branch from 448bf55 to 37d9568 Compare April 6, 2017 23:37

soumith closed this Apr 6, 2017

soumith reopened this Apr 6, 2017

soumith merged commit 37d9568 into pytorch:master Apr 6, 2017

albanD deleted the cwrap_wrap_dim branch April 7, 2017 09:26

albanD mentioned this pull request Apr 8, 2017

cat() using negative dimension #1213

Closed

ezyang added the open source label Jun 24, 2019

jjsjann123 pushed a commit to jjsjann123/pytorch that referenced this pull request Nov 5, 2021

Unswitch predicate equality fix (pytorch#1108)

b44a83b

unswitch predicate equality fix

janeyx99 mentioned this pull request Sep 6, 2022

Dr. CI bot is triggered on extremely old PRs pytorch/test-infra#668

Closed

hubertlu-tw pushed a commit to hubertlu-tw/pytorch that referenced this pull request Nov 1, 2022

compile cublasLt code only for cublas >= 11.0 (pytorch#1108)

00c1e56

Co-authored-by: Sukru Eryilmaz <seryilmaz@computelab-dgx1v-32.nvidia.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow support for negative dimension argument for all functions #1108

Allow support for negative dimension argument for all functions #1108

albanD commented Mar 26, 2017

apaszke left a 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.

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.

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.

soumith commented Apr 6, 2017

pytorch-bot bot commented Sep 1, 2022



		def idx_tensor(size, max_val):
		return torch.LongTensor(*size).random_() % max_val

Allow support for negative dimension argument for all functions #1108

Allow support for negative dimension argument for all functions #1108

Conversation

albanD commented Mar 26, 2017

apaszke 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.

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.

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.

soumith commented Apr 6, 2017

pytorch-bot bot commented Sep 1, 2022

🔗 Helpful Links

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