Relax int type restriction #3466

okuta · 2017-09-27T18:50:25Z

Current int type restriction is too strict.
This PR is relax it.

niboshi · 2017-09-28T13:43:41Z

chainer/functions/connection/embed_id.py

                    '''
                    if (x != ignore) {
-                      int w_ind[] = {x, i % n_out};
+                      ptrdiff_t w_ind[] = {x, i % n_out};


Ditto. (ptrdiff_t -> S)

niboshi · 2017-09-28T13:43:58Z

chainer/functions/evaluation/binary_accuracy.py

@@ -45,7 +43,7 @@ def binary_accuracy(y, t):

        t (:class:`~chainer.Variable` or :class:`numpy.ndarray` or \
        :class:`cupy.ndarray`):
-            Array holding an int32 vector of ground truth labels.
+            Array holding an signed integer vector of ground truth labels.


niboshi · 2017-09-28T13:44:15Z

chainer/functions/loss/sigmoid_cross_entropy.py

@@ -75,8 +75,8 @@ def sigmoid_cross_entropy(
        x (Variable): A variable object holding a matrix whose (i, j)-th
            element indicates the unnormalized log probability of the j-th unit
            at the i-th example.
-        t (Variable): Variable holding an int32 vector of ground truth labels.
-            If ``t[i] == -1``, corresponding ``x[i]`` is ignored.
+        t (Variable): Variable holding an signed integer vector of ground truth


ditto. (an -> a)

niboshi · 2017-09-28T13:44:41Z

chainer/functions/loss/softmax_cross_entropy.py

@@ -294,7 +294,7 @@ def softmax_cross_entropy(
            dimensions is greater than 2.
        t (:class:`~chainer.Variable` or :class:`numpy.ndarray` or \
        :class:`cupy.ndarray`):
-            Variable holding an :class:`numpy.int32` vector of ground truth
+            Variable holding an signed integer vector of ground truth


ditto. (an -> a)

niboshi · 2017-09-28T13:44:58Z

tests/chainer_tests/functions_tests/array_tests/test_permutate.py

+     {'shape': (3,), 'dtype': 'f', 'axis': -1, 'inv': True},
+     {'shape': (3, 4), 'dtype': 'd', 'axis': 1, 'inv': True},
+     {'shape': (3, 4, 5), 'dtype': 'f', 'axis': 2, 'inv': False}],
+    [{'label_dtype': numpy.int}, {'label_dtype': numpy.int32}]


How about other int types (int8, int16, int64)?

niboshi · 2017-09-28T13:46:05Z

tests/chainer_tests/functions_tests/loss_tests/test_hinge.py

+@testing.parameterize(*testing.product({
+    'reduce': ['no', 'mean'],
+    'norm': ['L1', 'L2'],
+    'label_dtype': [numpy.int, numpy.int32],


Ditto. (other int types)

niboshi · 2017-09-28T13:47:17Z

tests/chainer_tests/functions_tests/loss_tests/test_sigmoid_cross_entropy.py

    # too large shape causes int32 -> float64 issue
-    {'shape': (65536, 1), 'normalize': False},
+    {'shape': (65536, 1), 'normalize': False, 'label_dtype': numpy.int32},
+    {'shape': (8, 7), 'normalize': True, 'label_dtype': numpy.int}


How about adding some cases to test other int types?

niboshi · 2017-09-28T13:47:34Z

tests/chainer_tests/functions_tests/loss_tests/test_softmax_cross_entropy.py

@@ -21,6 +21,7 @@
    'dtype': [numpy.float32],
    'weight_apply': [False, True],
    'enable_double_backprop': [False, True],
+    'label_dtype': [numpy.int32],


Ditto. (other int types)

niboshi · 2017-09-28T13:47:39Z

tests/chainer_tests/functions_tests/loss_tests/test_softmax_cross_entropy.py

@@ -29,6 +30,7 @@
    'dtype': [numpy.float16, numpy.float32, numpy.float64],
    'weight_apply': [False, True],
    'enable_double_backprop': [False, True],
+    'label_dtype': [numpy.int, numpy.int32],


Ditto. (other int types)

niboshi · 2017-09-28T13:48:21Z

chainer/functions/connection/embed_id.py

-                    'T gy, int32 x, int32 n_out', 'raw T gW',
-                    'int w_ind[] = {x, i % n_out}; atomicAdd(&gW[w_ind], gy)',
+                    'T gy, S x, S n_out', 'raw T gW',
+                    'ptrdiff_t w_ind[] = {x, i % n_out};'


How about ptrdiff_t -> S? It's guaranteed to be representable by S.

ptrdiff_t is better for CuPy.

OK, I agree with you because it's used as an index.

niboshi · 2017-10-04T11:15:45Z

Please fix AppVeyor result.

okuta · 2017-10-14T13:16:36Z

I fixed.

okuta · 2017-10-21T17:40:22Z

I resolved conflict.

niboshi · 2017-10-23T01:46:19Z

chainer/functions/evaluation/classification_summary.py

@@ -43,6 +42,7 @@ def check_type_forward(self, in_types):
    def forward(self, inputs):
        xp = cuda.get_array_module(*inputs)
        y, t = inputs
+        t = t.astype('i', copy=False)


Is this conversion necessary?
I think the actual type for 'i' is environment-dependent.

Yes. Some Numpy functions require int32 on Windows.

I see. Can you write a brief comment on this line?

niboshi · 2017-10-24T02:33:45Z

Please fix conflict.

okuta · 2017-10-24T08:15:22Z

I resolved conflict.

niboshi · 2017-10-24T10:22:17Z

(Jenkins has passed. Waiting for travis)

niboshi · 2017-10-27T01:05:50Z

LGTM!

Relax int type restriction

niboshi requested changes Sep 28, 2017

View reviewed changes

niboshi self-assigned this Sep 28, 2017

niboshi added cat:enhancement Implementation that does not break interfaces. to-be-backported Pull request that should be backported. st:awaiting-author State indicating that response is needed from contributors, often authors of pull request. labels Sep 28, 2017

niboshi approved these changes Oct 1, 2017

View reviewed changes

niboshi removed the st:awaiting-author State indicating that response is needed from contributors, often authors of pull request. label Oct 1, 2017

niboshi added the st:awaiting-author State indicating that response is needed from contributors, often authors of pull request. label Oct 4, 2017

okuta force-pushed the support-int-type branch from 979b518 to b9840bf Compare October 14, 2017 12:59

niboshi added st:ready-for-review and removed st:awaiting-author State indicating that response is needed from contributors, often authors of pull request. labels Oct 16, 2017

okuta force-pushed the support-int-type branch from b9840bf to 10cc5b6 Compare October 21, 2017 17:40

niboshi requested changes Oct 23, 2017

View reviewed changes

niboshi approved these changes Oct 23, 2017

View reviewed changes

niboshi added st:test-and-merge State indicating that pull request is approved by a reviewer and can be merged after CI passes. and removed st:ready-for-review labels Oct 23, 2017

okuta added 5 commits October 24, 2017 16:00

Relax int type restriction

ba16ddf

Fix based on comments

785f3dc

Remove take

38d0672

Add cast

989d31b

Write comment

3352886

okuta force-pushed the support-int-type branch from b3ca2cd to 3352886 Compare October 24, 2017 07:01

niboshi added this to the v4.0.0b1 milestone Oct 27, 2017

niboshi merged commit 97ed424 into chainer:master Oct 27, 2017

niboshi added a commit to niboshi/chainer that referenced this pull request Oct 27, 2017

Merge pull request chainer#3466 from okuta/support-int-type

7b908fa

Relax int type restriction

niboshi mentioned this pull request Oct 27, 2017

[backport] Relax int type restriction #3700

Merged

niboshi added a commit to niboshi/chainer that referenced this pull request Oct 27, 2017

Merge pull request chainer#3466 from okuta/support-int-type

a8b1dbe

Relax int type restriction

okuta deleted the support-int-type branch February 13, 2018 01:25

kmaehashi mentioned this pull request Apr 23, 2018

chainer codes that correctly run in the windows but in ubuntu has error #3288

Closed

takagi mentioned this pull request Jul 25, 2018

Trivial fix for parameterized test case of F.contrastive #5147

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Relax int type restriction #3466

Relax int type restriction #3466

okuta commented Sep 27, 2017

niboshi Sep 28, 2017

okuta Oct 1, 2017

niboshi Sep 28, 2017

niboshi Sep 28, 2017 •

edited

niboshi Sep 28, 2017

niboshi Sep 28, 2017

niboshi Sep 28, 2017

niboshi Sep 28, 2017

niboshi Sep 28, 2017

niboshi Sep 28, 2017

niboshi Sep 28, 2017

okuta Oct 1, 2017

niboshi Oct 1, 2017

niboshi commented Oct 4, 2017

okuta commented Oct 14, 2017

okuta commented Oct 21, 2017

niboshi Oct 23, 2017

okuta Oct 23, 2017

niboshi Oct 23, 2017

okuta Oct 23, 2017

niboshi commented Oct 24, 2017

okuta commented Oct 24, 2017

niboshi commented Oct 24, 2017

niboshi commented Oct 27, 2017

Relax int type restriction #3466

Relax int type restriction #3466

Conversation

okuta commented Sep 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niboshi Sep 28, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niboshi commented Oct 4, 2017

okuta commented Oct 14, 2017

okuta commented Oct 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niboshi commented Oct 24, 2017

okuta commented Oct 24, 2017

niboshi commented Oct 24, 2017

niboshi commented Oct 27, 2017

niboshi Sep 28, 2017 •

edited