Enhance ops to support LoD as input for dygraph detection models. #25316

jerrywgz · 2020-07-01T11:26:22Z

PR types

Function optimization

PR changes

OPs

Describe

Enhance ops related to object detection to support LoD as input. Add RoIsNum for input and output which are dispensable and compatible with inference of previous Paddle version. These new input and output are only used in dygraph detection models which are not released yet.

These ops add new input and output and faster_rcnn_r50_fpn_1x has been verified that the latest paddle version could use the inference model from previous paddle version correctly.

预测兼容性测试：

测试方法：
在PaddleDetection中使用 export_model.py 导出模型，使用deploy/python/infer.py进行预测，对比检测结果。测试模型为faster_rcnn_r50_fpn_1x 其中包含了所有改动涉及到的op

case1：
使用paddle1.8.4版本保存模型，在1.8.4版本和develop版本分别预测
1.8.4版本预测结果：

develop版本预测结果：

case2:
使用develop版本保存模型，在1.8.4版本和develop版本分别预测
1.8.4版本预测结果：

develop版本预测结果：

测试结论：
新增输入输出是兼容老版本的

paddle-bot-old · 2020-07-01T11:26:27Z

Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

qingqing01 · 2020-08-05T03:18:52Z

paddle/fluid/operators/detection/collect_fpn_proposals_op.cc

+    AddInput(
+        "MultiLevelNums",
+        "(Tensor) Multiple RoIs number of each image from each level in shape"
+        "(N), N is the number of images.")


comments不通顺。

qingqing01 · 2020-08-05T04:02:52Z

paddle/fluid/operators/detection/collect_fpn_proposals_op.h

+        const int* cur_rois_num = multi_rois_num[i]->data<int>();
+        for (int k = 0; k < multi_rois_num[i]->numel(); k++) {
+          all_rois += cur_rois_num[k];
+        }


you also can use std::accumulate

qingqing01 · 2020-08-05T04:16:19Z

paddle/fluid/operators/detection/distribute_fpn_proposals_op.cc

    AddOutput("MultiFpnRois", "(LoDTensor) Output with distribute operator")
        .AsDuplicable();
    AddOutput("RestoreIndex",
              "(Tensor) An array of positive number which is "
              "used to restore the order of FpnRois");
+    AddOutput("MultiRoisNum",
+              "(Tensor) Multiple number of RoIs from each level in shape (B),"


这是一个List of Tensor吧，而不是一个Tensor

qingqing01 · 2020-08-05T04:19:08Z

paddle/fluid/operators/detection/distribute_fpn_proposals_op.cc

+      std::vector<framework::DDim> outs_num_dims;
+      for (size_t i = 0; i < num_out_rois; ++i) {
+        framework::DDim out_num_dim = {-1};
+        outs_num_dims.push_back(out_num_dim);


55行可以去掉， 56行: push_back({-1});

qingqing01 · 2020-08-05T04:21:00Z

paddle/fluid/operators/detection/distribute_fpn_proposals_op.h

@@ -28,6 +28,21 @@ namespace operators {

 const int kBoxDim = 4;

+inline std::vector<size_t> get_lod_from_rois_num(const Tensor* rois_num) {


Note code style for function-name https://google.github.io/styleguide/cppguide.html#Function_Names

qingqing01 · 2020-08-05T04:24:00Z

python/paddle/fluid/layers/detection.py

@@ -3562,6 +3564,12 @@ def distribute_fpn_proposals(fpn_rois,
        name(str, optional): For detailed information, please refer 
            to :ref:`api_guide_Name`. Usually name is no need to set and 
            None by default. 
+        rois_num(Variable): 1-D Tensor with shape [B] and data type is int32.
+            B is the number os images. The number of RoIs in each image.
+        return_rois_num(bool): When setting True, it will return a list 


当输入包含rois_num时，是否一定会返回，如果是，可以去掉这个bool控制

qingqing01 · 2020-08-05T04:28:16Z

python/paddle/fluid/layers/detection.py

@@ -3574,6 +3582,10 @@ def distribute_fpn_proposals(fpn_rois,
        the number of total rois. The data type is int32. It is
        used to restore the order of fpn_rois.

+        multi_rois_num(List): A list of 1-D Tensor with shape [B]
+        and data type of int32. B is the number of images. The number of RoIs 
+        in each image from each level.


最后一句没有谓语

qingqing01 · 2020-08-05T04:29:31Z

python/paddle/fluid/layers/detection.py

@@ -3720,14 +3745,19 @@ def collect_fpn_proposals(multi_rois,
        post_nms_top_n(int): The number of selected RoIs
        name(str, optional): For detailed information, please refer 
            to :ref:`api_guide_Name`. Usually name is no need to set and 
-            None by default.        
+            None by default.
+        multi_rois_num(list, optional): List of the number of RoIs in each image from each level. Element in list is 1-D Tensor with shape [B] and data type is int32, B is the number of images. Default: None


同样，第一句无法不通顺，没有谓语。 multi_rois_num 这个名字觉得不是很直观

修改为rois_num_per_level

jerrywgz · 2020-08-20T06:26:05Z

liym27 · 2020-08-20T06:44:58Z

paddle/fluid/operators/detection/generate_proposals_op.cc

@@ -481,7 +491,8 @@ class GenerateProposalsOpMaker : public framework::OpProtoAndCheckerMaker {
              "(LoDTensor), Output proposals with shape (rois_num, 4).");
    AddOutput("RpnRoiProbs",
              "(LoDTensor) Scores of proposals with shape (rois_num, 1).");
-    AddOutput("RpnRoisLod", "(Tensor), rpn rois's lod info").AsDispensable();
+    AddOutput("RpnRoisNum", "(Tensor), The number of Rpn RoIs in each image")
+        .AsDispensable();


为什么将输出变量RpnRoisLod删除，并新增RpnRoisNum呢？
为了保证新版本的Paddle预测库能成功加载旧版本训练的模型，当前要求op的输入输出只能做兼容性修改，不能删除已有的输入输出~
可参考：https://github.com/PaddlePaddle/Paddle/wiki/OP-Input-Output-Attribute-Compatibility-Modification

关于删除RoisLod相关输入输出问题统一回复：为多个相关api, op的接口保持统一，而且实际输入的值的含义并不是lod而是num。修改后的op主要是为动态图(不支持lod的地方用的)，目前release的模型都是静态图版本。同时这些op都是检测模型专有op，其他场景也基本不会用到。

liym27 · 2020-08-20T06:50:46Z

paddle/fluid/operators/roi_align_op.cc

             "(Tensor), "
-             "The lod info of rois.")
+             "The number of RoIs in each image.")


同上，会引入预测库不兼容风险

liym27 · 2020-08-20T06:51:01Z

paddle/fluid/operators/roi_pool_op.cc

@@ -140,7 +141,8 @@ class ROIPoolOpMaker : public framework::OpProtoAndCheckerMaker {
             "Where batch_id is the id of the data, "
             "(x1, y1) is the top left coordinates, and "
             "(x2, y2) is the bottom right coordinates.");
-    AddInput("RoisLod", "(Tensor), The lod info of rois.").AsDispensable();
+    AddInput("RoisNum", "(Tensor), The number of RoIs in each image.")
+        .AsDispensable();


同上，会引入预测库不兼容风险

zhiqiu · 2020-08-24T03:57:28Z

python/paddle/fluid/layers/detection.py

+            for i in range(num_lvl)
+        ]
+        outputs['MultiLevelRoIsNum'] = rois_num_per_level
+
    helper.append_op(


Call core.ops.distribute_fpn_proposals here for better performance in dygraph mode.

zhiqiu

LGTM for op_function_generator.cc

zhiqiu · 2020-09-01T03:29:46Z

python/paddle/fluid/layers/detection.py

+    num_lvl = max_level - min_level + 1
+
+    if in_dygraph_mode():
+        assert rois_num is not None, "rois_num should not be None in dygraph mode."


I suggest adding the reason why rois_num should not be None in error message.
Otherwise, users may be confused when seeing the error.

Not urgent, you can refine it in the next PR. Same for other APIs.

Heeenrrry

LGTM

Superjomn

LGTM

zhiqiu

LGTM

Heeenrrry

LGTM

XiaoguangHu01

LGTM

enhance collect_op for dygraph, test=develop

792d51f

jerrywgz force-pushed the enhance_fpn_ops branch from a517d3d to 792d51f Compare July 1, 2020 11:35

jerrywgz changed the title ~~enhance collect_op for dygraph, test=develop~~ enhance collect_fpn_proposals_op for dygraph, test=develop Jul 1, 2020

jerrywgz changed the title ~~enhance collect_fpn_proposals_op for dygraph, test=develop~~ enhance ops for dygraph in FPN models, test=develop Jul 2, 2020

jerrywgz force-pushed the enhance_fpn_ops branch from 22978a1 to b437aea Compare August 2, 2020 07:31

enhance detection ops with lod, test=develop

f66a5a3

jerrywgz force-pushed the enhance_fpn_ops branch from b437aea to f66a5a3 Compare August 2, 2020 14:48

qingqing01 reviewed Aug 5, 2020

View reviewed changes

jerrywgz added 2 commits August 6, 2020 13:13

update code & doc, test=develop

fe149b8

support none bbox left in generate_proposals, test=develop

c950b9d

jerrywgz requested review from yghstill and qingqing01 August 13, 2020 06:22

qingqing01 previously approved these changes Aug 17, 2020

View reviewed changes

jerrywgz dismissed qingqing01’s stale review via 5fe372b August 18, 2020 11:16

jerrywgz force-pushed the enhance_fpn_ops branch from 54ebaf3 to 5fe372b Compare August 18, 2020 11:16

unfiy MultiLevelRoisNum, test=develop

ac03701

jerrywgz force-pushed the enhance_fpn_ops branch 2 times, most recently from 213096f to 8df7a50 Compare August 19, 2020 11:07

update interface for inference, test=develop

e690dc0

jerrywgz force-pushed the enhance_fpn_ops branch from 8df7a50 to e690dc0 Compare August 19, 2020 12:11

qingqing01 changed the title ~~enhance ops for dygraph in FPN models, test=develop~~ Enhance ops to support LoD as input for dygraph detection models. Aug 20, 2020

qingqing01 previously approved these changes Aug 20, 2020

View reviewed changes

jerrywgz requested review from liym27, Heeenrrry and zhiqiu August 20, 2020 06:27

liym27 reviewed Aug 20, 2020

View reviewed changes

fix test_layer, test=develop

dbb9b99

jerrywgz dismissed qingqing01’s stale review via dbb9b99 August 20, 2020 09:02

zhiqiu reviewed Aug 24, 2020

View reviewed changes

jerrywgz added 2 commits August 28, 2020 12:34

update core.ops, test=develop

32cb889

update upstream, test=develop

f098394

zhiqiu previously approved these changes Sep 1, 2020

View reviewed changes

Heeenrrry previously approved these changes Sep 1, 2020

View reviewed changes

add op register for new input & output, test=develop

ce62886

jerrywgz dismissed stale reviews from Heeenrrry and zhiqiu via ce62886 September 2, 2020 11:24

jerrywgz requested a review from Superjomn September 7, 2020 02:17

Superjomn approved these changes Sep 7, 2020

View reviewed changes

zhiqiu self-requested a review September 7, 2020 02:49

zhiqiu approved these changes Sep 7, 2020

View reviewed changes

Heeenrrry approved these changes Sep 7, 2020

View reviewed changes

XiaoguangHu01 approved these changes Sep 7, 2020

View reviewed changes

raindrops2sea approved these changes Sep 8, 2020

View reviewed changes

jerrywgz merged commit a28ae86 into PaddlePaddle:develop Sep 8, 2020

jerrywgz deleted the enhance_fpn_ops branch September 8, 2020 11:06

jerrywgz mentioned this pull request Nov 3, 2020

operator < generate_proposals > error: The index of gather_op should not be empty when the index's rank is 1 PaddlePaddle/PaddleDetection#990

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance ops to support LoD as input for dygraph detection models. #25316

Enhance ops to support LoD as input for dygraph detection models. #25316

jerrywgz commented Jul 1, 2020 •

edited

Loading

paddle-bot-old bot commented Jul 1, 2020

qingqing01 Aug 5, 2020

jerrywgz Aug 6, 2020

qingqing01 Aug 5, 2020

jerrywgz Aug 6, 2020

qingqing01 Aug 5, 2020

jerrywgz Aug 6, 2020

qingqing01 Aug 5, 2020

jerrywgz Aug 6, 2020

qingqing01 Aug 5, 2020

jerrywgz Aug 6, 2020

qingqing01 Aug 5, 2020

jerrywgz Aug 6, 2020

qingqing01 Aug 5, 2020

jerrywgz Aug 6, 2020

qingqing01 Aug 5, 2020

jerrywgz Aug 6, 2020

jerrywgz commented Aug 20, 2020

liym27 Aug 20, 2020

jerrywgz Aug 20, 2020 •

edited

Loading

liym27 Aug 20, 2020

liym27 Aug 20, 2020

zhiqiu Aug 24, 2020

jerrywgz Sep 1, 2020

zhiqiu left a comment

zhiqiu Sep 1, 2020

Heeenrrry left a comment

Superjomn left a comment

zhiqiu left a comment

Heeenrrry left a comment

XiaoguangHu01 left a comment

		@@ -28,6 +28,21 @@ namespace operators {

		const int kBoxDim = 4;

		inline std::vector<size_t> get_lod_from_rois_num(const Tensor* rois_num) {

Enhance ops to support LoD as input for dygraph detection models. #25316

Enhance ops to support LoD as input for dygraph detection models. #25316

Conversation

jerrywgz commented Jul 1, 2020 • edited Loading

PR types

PR changes

Describe

paddle-bot-old bot commented Jul 1, 2020

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

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

jerrywgz commented Aug 20, 2020

Choose a reason for hiding this comment

jerrywgz Aug 20, 2020 • edited Loading

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

zhiqiu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Heeenrrry left a comment

Choose a reason for hiding this comment

Superjomn left a comment

Choose a reason for hiding this comment

zhiqiu left a comment

Choose a reason for hiding this comment

Heeenrrry left a comment

Choose a reason for hiding this comment

XiaoguangHu01 left a comment

Choose a reason for hiding this comment

jerrywgz commented Jul 1, 2020 •

edited

Loading

jerrywgz Aug 20, 2020 •

edited

Loading