From d19355a53221e43ff93b036433cef101cfc30821 Mon Sep 17 00:00:00 2001
From: guosheng <guosheng@baidu.com>
Date: Wed, 2 Aug 2017 15:50:50 +0800
Subject: [PATCH] Refine ClipLayer and add unit test for it

---
 doc/api/v2/config/layer.rst                   |  5 ++++
 paddle/gserver/layers/ClipLayer.cpp           | 29 ++++++++++---------
 paddle/gserver/tests/test_LayerGrad.cpp       |  4 +--
 proto/ModelConfig.proto                       |  4 +--
 python/paddle/trainer/config_parser.py        | 15 ++++------
 .../paddle/trainer_config_helpers/layers.py   | 16 +++++-----
 .../tests/configs/file_list.sh                |  2 +-
 .../tests/configs/test_clip_layer.py          |  6 ++++
 8 files changed, 45 insertions(+), 36 deletions(-)
 create mode 100644 python/paddle/trainer_config_helpers/tests/configs/test_clip_layer.py

diff --git a/doc/api/v2/config/layer.rst b/doc/api/v2/config/layer.rst
index daee55b7f9..d7eff17734 100644
--- a/doc/api/v2/config/layer.rst
+++ b/doc/api/v2/config/layer.rst
@@ -316,6 +316,11 @@ scaling
 ..  autoclass:: paddle.v2.layer.scaling
     :noindex:
 
+clip
+----
+..  autoclass:: paddle.v2.layer.clip
+    :noindex:
+
 slope_intercept
 ---------------
 ..  autoclass:: paddle.v2.layer.slope_intercept
diff --git a/paddle/gserver/layers/ClipLayer.cpp b/paddle/gserver/layers/ClipLayer.cpp
index 51f0e0d2f0..13f16c9537 100644
--- a/paddle/gserver/layers/ClipLayer.cpp
+++ b/paddle/gserver/layers/ClipLayer.cpp
@@ -13,7 +13,6 @@ See the License for the specific language governing permissions and
 limitations under the License. */
 
 #include "Layer.h"
-#include "paddle/math/Matrix.h"
 
 namespace paddle {
 
@@ -26,8 +25,8 @@ namespace paddle {
 
 class ClipLayer : public Layer {
 protected:
-  real clipThresholdLow_;
-  real clipThresholdHigh_;
+  double min_;
+  double max_;
 
 public:
   explicit ClipLayer(const LayerConfig& config) : Layer(config) {}
@@ -47,9 +46,9 @@ bool ClipLayer::init(const LayerMap& layerMap,
 
   CHECK_EQ(inputLayers_.size(), 1U);
   auto layerConf = config_.inputs(0).clip_conf();
-  clipThresholdLow_ = layerConf.clip_threshold_low();
-  clipThresholdHigh_ = layerConf.clip_threshold_high();
-  CHECK_LT(clipThresholdLow_, clipThresholdHigh_);
+  min_ = layerConf.min();
+  max_ = layerConf.max();
+  CHECK_LT(min_, max_);
   return true;
 }
 
@@ -60,19 +59,21 @@ void ClipLayer::forward(PassType passType) {
   resetOutput(inV->getHeight(), inV->getWidth());
   MatrixPtr outV = getOutputValue();
   outV->copyFrom(*inV);
-  outV->clip(clipThresholdLow_, clipThresholdHigh_);
+  outV->clip(min_, max_);
 }
 
 void ClipLayer::backward(const UpdateCallback& callback) {
   MatrixPtr inV = getInputValue(0);
   MatrixPtr inG = getInputGrad(0);
-  MatrixPtr outV = getOutputValue();
-  MatrixPtr outG = getOutputGrad();
-  MatrixPtr tmpMtx;
-  Matrix::resizeOrCreate(
-      tmpMtx, outG->getHeight(), outG->getWidth(), false, useGpu_);
-  tmpMtx->clipDerivative(*inV, clipThresholdLow_, clipThresholdHigh_);
-  inG->addDotMul(*outG, *tmpMtx, 1, 1);
+  if (inG) {
+    MatrixPtr outV = getOutputValue();
+    MatrixPtr outG = getOutputGrad();
+    MatrixPtr tmpMtx;
+    Matrix::resizeOrCreate(
+        tmpMtx, outG->getHeight(), outG->getWidth(), false, useGpu_);
+    tmpMtx->clipDerivative(*inV, min_, max_);
+    inG->addDotMul(*outG, *tmpMtx, 1, 1);
+  }
 }
 
 }  // namespace paddle
diff --git a/paddle/gserver/tests/test_LayerGrad.cpp b/paddle/gserver/tests/test_LayerGrad.cpp
index b0032adb39..f01bf3bc78 100644
--- a/paddle/gserver/tests/test_LayerGrad.cpp
+++ b/paddle/gserver/tests/test_LayerGrad.cpp
@@ -1887,8 +1887,8 @@ TEST(Layer, ClipLayer) {
   config.inputDefs.push_back({INPUT_DATA, "input", size, 0});
   LayerInputConfig* input = config.layerConfig.add_inputs();
   ClipConfig* layerConf = input->mutable_clip_conf();
-  layerConf->set_clip_threshold_low(std::rand() / (real)RAND_MAX);
-  layerConf->set_clip_threshold_high(std::rand() / (real)RAND_MAX);
+  layerConf->set_min(std::rand() / (double)RAND_MAX);
+  layerConf->set_max(std::rand() / (double)RAND_MAX);
   for (auto useGpu : {false, true}) {
     testLayerGrad(config, "clip", batchSize, false, useGpu, false);
   }
diff --git a/proto/ModelConfig.proto b/proto/ModelConfig.proto
index 772fc3c4ca..5ceb16a7b6 100644
--- a/proto/ModelConfig.proto
+++ b/proto/ModelConfig.proto
@@ -290,8 +290,8 @@ message DetectionOutputConfig {
 }
 
 message ClipConfig {
-  required float clip_threshold_low = 1;
-  required float clip_threshold_high = 2;
+  required double min = 1;
+  required double max = 2;
 }
 
 message LayerInputConfig {
diff --git a/python/paddle/trainer/config_parser.py b/python/paddle/trainer/config_parser.py
index 9b2e9ea784..637f70f39c 100644
--- a/python/paddle/trainer/config_parser.py
+++ b/python/paddle/trainer/config_parser.py
@@ -2171,19 +2171,16 @@ class RowConvLayer(LayerBase):
 
 @config_layer('clip')
 class ClipLayer(LayerBase):
-    def __init__(self, name, inputs, clip_threshold_low, clip_threshold_high):
-        super(ClipLayer, self).__init__(name, 'clip', 0, inputs=inputs)
+    def __init__(self, name, inputs, min, max, **xargs):
+        super(ClipLayer, self).__init__(name, 'clip', 0, inputs=inputs, **xargs)
         config_assert(
             len(self.inputs) == 1,
-            'ClipLayer layer must have one and only one input.')
-        config_assert(
-            clip_threshold_low < clip_threshold_high,
-            'clip_threshold_low must be less than clip_threshold_high.')
+            'ClipLayer must have one and only one input.')
+        config_assert(min < max, 'min must be less than max.')
         input_layer = self.get_input_layer(0)
         self.set_layer_size(input_layer.size)
-        self.config.inputs[0].clip_conf.clip_threshold_low = clip_threshold_low
-        self.config.inputs[
-            0].clip_conf.clip_threshold_high = clip_threshold_high
+        self.config.inputs[0].clip_conf.min = min
+        self.config.inputs[0].clip_conf.max = max
 
 
 # key: cost type
diff --git a/python/paddle/trainer_config_helpers/layers.py b/python/paddle/trainer_config_helpers/layers.py
index bd79bf66b0..33a7fdb3da 100755
--- a/python/paddle/trainer_config_helpers/layers.py
+++ b/python/paddle/trainer_config_helpers/layers.py
@@ -6011,7 +6011,7 @@ def crop_layer(input, offset, axis=2, shape=None, name=None, layer_attr=None):
 
 
 @wrap_name_default("clip")
-def clip_layer(input, clip_threshold_low, clip_threshold_high, name=None):
+def clip_layer(input, min, max, name=None):
     """
     A layer for clipping the input value by the threshold.
 
@@ -6021,23 +6021,23 @@ def clip_layer(input, clip_threshold_low, clip_threshold_high, name=None):
 
     .. code-block:: python
 
-        clip = clip_layer(input=input_layer, clip_threshold_low=-10, clip_threshold_high=10)
+        clip = clip_layer(input=input_layer, min=-10, max=10)
 
     :param name: The Layer Name.
     :type name: basestring
     :param input: The input layer.
     :type input: LayerOutput.
-    :param clip_threshold_low: The lower threshold for clipping.
-    :type clip_threshold_low: float
-    :param clip_threshold_high: The upper threshold for clipping.
-    :type clip_threshold_high: float
+    :param min: The lower threshold for clipping.
+    :type min: double
+    :param max: The upper threshold for clipping.
+    :type max: double
     :return: LayerOutput
     """
     Layer(
         name=name,
         type=LayerType.CLIP_LAYER,
         inputs=[input.name],
-        clip_threshold_low=clip_threshold_low,
-        clip_threshold_high=clip_threshold_high)
+        min=min,
+        max=max)
     return LayerOutput(
         name, LayerType.CLIP_LAYER, parents=[input], size=input.size)
diff --git a/python/paddle/trainer_config_helpers/tests/configs/file_list.sh b/python/paddle/trainer_config_helpers/tests/configs/file_list.sh
index cdf9b2eab7..d71c41f77e 100755
--- a/python/paddle/trainer_config_helpers/tests/configs/file_list.sh
+++ b/python/paddle/trainer_config_helpers/tests/configs/file_list.sh
@@ -7,6 +7,6 @@ test_rnn_group shared_fc shared_lstm shared_gru test_cost_layers_with_weight
 test_spp_layer test_bilinear_interp test_maxout test_bi_grumemory math_ops
 test_seq_concat_reshape test_pad test_smooth_l1 test_multiplex_layer
 test_prelu_layer test_row_conv test_detection_output_layer test_multibox_loss_layer
-test_recursive_topology test_gated_unit_layer)
+test_recursive_topology test_gated_unit_layer test_clip_layer)
 
 export whole_configs=(test_split_datasource)
diff --git a/python/paddle/trainer_config_helpers/tests/configs/test_clip_layer.py b/python/paddle/trainer_config_helpers/tests/configs/test_clip_layer.py
new file mode 100644
index 0000000000..f066fe1fb3
--- /dev/null
+++ b/python/paddle/trainer_config_helpers/tests/configs/test_clip_layer.py
@@ -0,0 +1,6 @@
+from paddle.trainer_config_helpers import *
+
+data = data_layer(name='input', size=300)
+clip = clip_layer(input=data, min=-10, max=10)
+
+outputs(clip)
-- 
GitLab