pass unit test for margin_rank_loss_op

paddle/operators/margin_rank_loss_op.cc

@@ -19,11 +19,7 @@ namespace operators {
 
 class MarginRankLossOp : public framework::OperatorWithKernel {
  public:
-  MarginRankLossOp(const std::string &type,
-                   const framework::VariableNameMap &inputs,
-                   const framework::VariableNameMap &outputs,
-                   const framework::AttributeMap &attrs)
-      : OperatorWithKernel(type, inputs, outputs, attrs) {}
+  using framework::OperatorWithKernel::OperatorWithKernel;
 
  protected:
   void InferShape(const framework::InferShapeContext &ctx) const override {
@@ -35,13 +31,11 @@ class MarginRankLossOp : public framework::OperatorWithKernel {
     auto label_dims = ctx.Input<framework::Tensor>("Label")->dims();
     auto x1_dims = ctx.Input<framework::Tensor>("X1")->dims();
     auto x2_dims = ctx.Input<framework::Tensor>("X2")->dims();
-    PADDLE_ENFORCE((label_dims.size() == 1) && (x1_dims.size() == 1) &&
-                       (x2_dims.size() == 1),
-                   "The rank of all inputs must be 1.");
-    PADDLE_ENFORCE((label_dims == x1_dims) && (x1_dims == x2_dims),
-                   "All inputs must have the same size");
-    ctx.Output<framework::LoDTensor>("Out")->Resize(label_dims);
+    PADDLE_ENFORCE((label_dims == x1_dims) && (x1_dims == x2_dims) &&
+                       (label_dims.size() == 2) && (label_dims[1] == 1),
+                   "All inputs must be vector with the same size");
     ctx.Output<framework::LoDTensor>("Activated")->Resize(label_dims);
+    ctx.Output<framework::LoDTensor>("Out")->Resize(label_dims);
   }
 };
 
@@ -51,18 +45,27 @@ class MarginRankLossOpMaker : public framework::OpProtoAndCheckerMaker {
   MarginRankLossOpMaker(framework::OpProto *proto,
                         framework::OpAttrChecker *op_checker)
       : OpProtoAndCheckerMaker(proto, op_checker) {
-    AddInput("Label", "The label indicating X1 ranked higher than X2 or not.");
-    AddInput("X1", "The first input of MarginRankLossOp.");
-    AddInput("X2", "The second input of MarginRankLossOp");
-    AddAttr<AttrType>("margin", "Margin for MarginRankLossOp").SetDefault(0);
-    AddOutput("Out", "The output loss of MarginRankLoss operator");
+    AddInput("X1", "The first input of MarginRankLossOp, row vector.");
+    AddInput("X2", "The second input of MarginRankLossOp, row vector.");
+    AddInput("Label",
+             "The label indicating X1 ranked higher than X2 "
+             "or not, row vector.");
+    AddAttr<AttrType>("margin", "Margin for MarginRankLossOp, scalar.")
+        .SetDefault(0);
     AddOutput("Activated",
-              "Intermediate tensor to indicate "
-              "whether Output(Out) is activated")
+              "Intermediate tensor to indicate whether each element of "
+              "Output(Out) is activated")
         .AsIntermediate();
-    AddComment(R"DOC(MarginRankLoss operator
+    AddOutput("Out", "The output loss of MarginRankLoss operator");
+    AddComment(R"DOC(
+
+MarginRankLoss operator measures the loss given a pair of input {`X1`, `X2`}
+and `Label` with attribuute `margin`, where `Label == 1` indicating X1 is
+ranked higher than `X2`, otherwise `Label == -1`. The loss turns out
+
+loss(X1, X2, Label) = max(0, -Label * (X1-X2) + margin)
 
-loss(x1, x2, y) = max(0, -label * (x1-x2) + margin)
+For batch input, `X1`, `X2` and `Label` all have the same size batch_size x 1.
 
 )DOC");
   }
@@ -70,11 +73,7 @@ loss(x1, x2, y) = max(0, -label * (x1-x2) + margin)
 
 class MarginRankLossGradOp : public framework::OperatorWithKernel {
  public:
-  MarginRankLossGradOp(const std::string &type,
-                       const framework::VariableNameMap &inputs,
-                       const framework::VariableNameMap &outputs,
-                       const framework::AttributeMap &attrs)
-      : OperatorWithKernel(type, inputs, outputs, attrs) {}
+  using framework::OperatorWithKernel::OperatorWithKernel;
 
  protected:
   void InferShape(const framework::InferShapeContext &ctx) const override {

paddle/operators/margin_rank_loss_op.cu

@@ -14,9 +14,11 @@
 
 #include "paddle/operators/margin_rank_loss_op.h"
 
+namespace ops = paddle::operators;
+
 REGISTER_OP_GPU_KERNEL(
     margin_rank_loss,
-    paddle::operators::MarginRankLossKernel<paddle::platform::GPUPlace, float>);
-REGISTER_OP_GPU_KERNEL(margin_rank_loss_grad,
-                       paddle::operators::MarginRankLossGradKernel<
-                           paddle::platform::GPUPlace, float>);
+    ops::MarginRankLossKernel<paddle::platform::GPUPlace, float>);
+REGISTER_OP_GPU_KERNEL(
+    margin_rank_loss_grad,
+    ops::MarginRankLossGradKernel<paddle::platform::GPUPlace, float>);

paddle/operators/margin_rank_loss_op.h

@@ -46,8 +46,8 @@ template <typename Place, typename T, typename AttrType = T>
 class MarginRankLossKernel : public framework::OpKernel {
  public:
   void Compute(const framework::ExecutionContext& ctx) const {
-    auto* out_t = ctx.Output<framework::LoDTensor>("Out");
-    auto* act_t = ctx.Output<framework::LoDTensor>("Activated");
+    auto* out_t = ctx.Output<framework::Tensor>("Out");
+    auto* act_t = ctx.Output<framework::Tensor>("Activated");
 
     auto* label_t = ctx.Input<framework::Tensor>("Label");
     auto* x1_t = ctx.Input<framework::Tensor>("X1");
@@ -65,8 +65,8 @@ class MarginRankLossKernel : public framework::OpKernel {
     auto x2 = framework::EigenVector<T>::Flatten(*x2_t);
 
     auto& dev = ctx.GetEigenDevice<Place>();
-    act.device(dev) = (-label * (x1 - x2) + margin).unaryExpr(Heaviside<T>());
     out.device(dev) = (-label * (x1 - x2) + margin).unaryExpr(ReLU<T>());
+    act.device(dev) = out.unaryExpr(Heaviside<T>());
   }
 };
 
@@ -78,15 +78,15 @@ class MarginRankLossGradKernel : public framework::OpKernel {
         ctx.Output<framework::LoDTensor>(framework::GradVarName("X1"));
     auto* d_x2_t =
         ctx.Output<framework::LoDTensor>(framework::GradVarName("X2"));
-    auto* act_t = ctx.Output<framework::LoDTensor>("Activated");
 
+    auto* act_t = ctx.Input<framework::Tensor>("Activated");
     auto* d_out_t = ctx.Input<framework::Tensor>(framework::GradVarName("Out"));
     auto* label_t = ctx.Input<framework::Tensor>("Label");
 
-    auto& dev = ctx.GetEigenDevice<Place>();
     auto d_out = framework::EigenVector<T>::Flatten(*d_out_t);
     auto act = framework::EigenVector<T>::Flatten(*act_t);
     auto label = framework::EigenVector<T>::Flatten(*label_t);
+    auto& dev = ctx.GetEigenDevice<Place>();
 
     // compute d_x1
     if (d_x1_t) {

python/paddle/v2/framework/tests/test_margin_rank_loss_op.py

@@ -8,23 +8,23 @@ class TestMarginRankLossOp(OpTest):
         self.op_type = "margin_rank_loss"
         batch_size = 5
         margin = 0.1
-        # labels_{i} = {0, 1.0} or {0, 0.5, 1.0}
-        label = np.random.randint(0, 2, size=(batch_size, )).astype("float32")
-        x1 = np.random.random((batch_size, )).astype("float32")
-        x2 = np.random.random((batch_size, )).astype("float32")
+        # labels_{i} = {-1, 1}
+        label = 2 * np.random.randint(
+            0, 2, size=(batch_size, 1)).astype("float32") - 1
+        x1 = np.random.random((batch_size, 1)).astype("float32")
+        x2 = np.random.random((batch_size, 1)).astype("float32")
         # loss = max(0, -label * (x1 - x2) + margin)
-        loss = [
-            max(0, -label[i] * (x1[i] - x2[i]) + margin)
-            for i in range(batch_size)
-        ]
+        loss = -label * (x1 - x2) + margin
+        loss = np.where(loss > 0, loss, 0)
+        act = np.where(loss > 0, 1., 0.)
+
         self.attrs = {'margin': margin}
         self.inputs = {'Label': label, 'X1': x1, 'X2': x2}
-        self.outputs = {'Out': loss}
+        self.outputs = {'Activated': act, 'Out': loss}
 
     def test_check_output(self):
         self.check_output()
 
-    """
     def test_check_grad(self):
         self.check_grad(["X1", "X2"], "Out")
 
@@ -33,7 +33,6 @@ class TestMarginRankLossOp(OpTest):
 
     def test_check_grad_ignore_x2(self):
         self.check_grad(["X1"], "Out", no_grad_set=set('X2'))
-    """
 
 
 if __name__ == '__main__':