add reduce=True argument to MultiLabelMarginLoss by li-roy · Pull Request #4924 · pytorch/pytorch

li-roy · 2018-01-30T00:54:08Z

As per #264. When reduce is False, MultiLabelMarginLoss outputs a loss per sample in minibatch. When reduce is True (default), the current behavior is kept.

Test Plan
test/run_test.sh
Added unit test. For the reduce=False case. Added unit tests for 1d tensors. Added a reference function.

pytorchbot · 2018-01-30T00:54:09Z

@li-roy, thanks for your PR! We identified @zdevito to be a potential reviewer.

li-roy · 2018-01-30T01:24:54Z

@pytorchbot retest this please

zou3519 · 2018-01-30T02:06:33Z

@pytorchbot retest this please

zou3519

Haven't reviewed the C code or kernels yet, will get to that soon. I left a few comments on the python side to start out

Sign in to view


 template <typename Dtype, typename Acctype>
 __global__ void cunn_MultiLabelMarginCriterion_updateGradInput_kernel(Dtype *gradInput,
+                                                                      Dtype *gradOutput,


Sign in to view

+            size_average. Default: True
+
+    Shape:
+        - Input: :math:`(N)` or :math:`(N, *)` where `*` means, any number of additional


Sign in to view

+        - Input: :math:`(N)` or :math:`(N, *)` where `*` means, any number of additional
+          dimensions
+        - Target: :math:`(N)` or :math:`(N, *)`, same shape as the input
+        - Output: scalar. If `reduce` is False, then `(N)` or `(N, *)`, same shape as


Sign in to view

@@ -565,10 +565,32 @@ class MultiLabelMarginLoss(_Loss):
    The criterion only considers the first non-negative `y[j]` targets.


Sign in to view

+
+
+def multilabelmarginloss_no_reduce_test():
+    t = Variable(torch.rand(5, 10).mul(10).floor().long())


Sign in to view

        check_no_size_average=True,
    ),
+    dict(
+        module_name='MultiLabelMarginLoss',


Sign in to view

+    if input.dim() == 1:
+        n = 1
+        dim = input.size()[0]
+        output = torch.Tensor(n).zero_()


Sign in to view

    return output


+def _multilabelmarginloss_reference(input, target, is_target):


Sign in to view

+
+    if input.dim() == 1:
+        n = 1
+        dim = input.size()[0]


Sign in to view

+def _multilabelmarginloss_reference(input, target, is_target):
+    sum = 0
+    for i in range(0, target.size()[0]):
+        target_index = target[i]


zou3519

The THNN code looks good for the most part! Just a few nits here and there. Haven't looked at the cuda kernel yet.

Sign in to view

    isTarget_data += dim;
    gradInput_data += dim;
  }
+  gradInput_data -= nframe*dim;


Sign in to view

    }
+
+    sum /= dim;
+    THTensor_(set1d)(output, t, sum);


Sign in to view

+  else
+  {
+    THNN_CHECK_DIM_SIZE(gradOutput, 1, 0, nframe);
+    gradOutput = THTensor_(newContiguous)(gradOutput);


Sign in to view

  THTensor_(free)(input);
  THIndexTensor_(free)(target);
  THTensor_(free)(isTarget);
+  THTensor_(free)(gradOutput);


Sign in to view

+        reduce (bool, optional): By default, the losses are averaged or summed over
+            observations for each minibatch depending on size_average. When reduce
+            is False, returns a loss per batch element instead and ignores
+            size_average. Default: True


Sign in to view

@@ -133,6 +140,11 @@ __global__ void cunn_MultiLabelMarginCriterion_updateGradInput_kernel(Dtype *gra
    }
    __syncthreads();


Sign in to view

+        reduce (bool, optional): By default, the losses are averaged or summed over
+            observations for each minibatch depending on size_average. When reduce
+            is False, returns a loss per batch element instead and ignores
+            size_average. Default: True


Sign in to view

+            size_average is set to ``False``, the losses are instead summed for
+            each minibatch. Default: ``True``
+        reduce (bool, optional): By default, the losses are averaged or summed over
+            observations for each minibatch depending on size_average. When reduce


Sign in to view

  THIndexTensor_(free)(target);
  THTensor_(free)(isTarget);
-  THTensor_(free)(gradOutput);
+  THTensor_(free)(gradInput);


zou3519

All comments addressed, LGTM unless anyone has something else to add. Thanks @li-roy!

ssnl · 2018-02-02T21:51:56Z

Thanks @li-roy !

zou3519 · 2018-02-02T21:55:13Z

I'm not sure why the CI isn't running, but... let's give this one final test

zou3519 · 2018-02-02T21:55:22Z

@pytorchbot retest this please

* add reduce=True argument to MultiLabelMarginLoss * Fix lint * Addressed comments * Remove unneeded syncthreads calls

add reduce=True argument to MultiLabelMarginLoss

b93ada4

Fix lint

212c331

li-roy closed this Jan 30, 2018

li-roy reopened this Jan 30, 2018

zou3519 reviewed Jan 31, 2018

View reviewed changes

Addressed comments

25ff953

ssnl reviewed Feb 1, 2018

View reviewed changes

zou3519 reviewed Feb 2, 2018

View reviewed changes

Remove unneeded syncthreads calls

11e85a0

zou3519 approved these changes Feb 2, 2018

View reviewed changes

soumith merged commit 28f056f into pytorch:master Feb 5, 2018

soumith added 0.3.1 and removed 0.3.1 labels Feb 5, 2018

li-roy deleted the multilabelmarginloss_reduce branch February 22, 2018 00:17

laurentdupin pushed a commit to laurentdupin/pytorch that referenced this pull request Apr 24, 2026

add reduce=True argument to MultiLabelMarginLoss (pytorch#4924)

b316422

* add reduce=True argument to MultiLabelMarginLoss * Fix lint * Addressed comments * Remove unneeded syncthreads calls

		@@ -565,10 +565,32 @@ class MultiLabelMarginLoss(_Loss):
		The criterion only considers the first non-negative `y[j]` targets.



		def multilabelmarginloss_no_reduce_test():
		t = Variable(torch.rand(5, 10).mul(10).floor().long())

		return output


		def _multilabelmarginloss_reference(input, target, is_target):

		@@ -133,6 +140,11 @@ __global__ void cunn_MultiLabelMarginCriterion_updateGradInput_kernel(Dtype *gra
		}
		__syncthreads();

Conversation

li-roy commented Jan 30, 2018

Uh oh!

pytorchbot commented Jan 30, 2018

Uh oh!

li-roy commented Jan 30, 2018

Uh oh!

zou3519 commented Jan 30, 2018

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as off-topic.

Uh oh!

zou3519 left a comment

Choose a reason for hiding this comment

Uh oh!

ssnl commented Feb 2, 2018

Uh oh!

zou3519 commented Feb 2, 2018

Uh oh!

zou3519 commented Feb 2, 2018

Uh oh!