Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support code generation for op multiclass_nms3 #54272

Merged
merged 9 commits into from
Jun 5, 2023

Conversation

huangjiyi
Copy link
Member

PR types

Others

PR changes

Others

Description

@paddle-bot
Copy link

paddle-bot bot commented Jun 1, 2023

你的PR提交成功,感谢你对开源项目的贡献!
请关注后续CI自动化测试结果,详情请参考Paddle-CI手册
Your PR has been submitted. Thanks for your contribution!
Please wait for the result of CI firstly. See Paddle CI Manual for details.

@paddle-bot paddle-bot bot added contributor External developers status: proposed labels Jun 1, 2023
@luotao1 luotao1 added the HappyOpenSource 快乐开源活动issue与PR label Jun 1, 2023
Copy link
Contributor

@heavyrain-lzy heavyrain-lzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

目前看PR没什么问题,py3的CI失败在编译blas库,可以尝试rerun

@huangjiyi
Copy link
Member Author

目前看PR没什么问题,py3的CI失败在编译blas库,可以尝试rerun

应该在生成阶段出的问题,我待会试着修一下

image

Comment on lines 1469 to 1478
# Get return type list & outputs
returns_type_list = ["" for i in range(num_outputs)]
returns_list = ["" for i in range(num_outputs)]
num_visited_intermediate_outputs = 0
for name, (rtype, pos) in forward_outputs_position_map.items():
if name in intermediate_outputs:
num_visited_intermediate_outputs += 1
continue
pos -= num_visited_intermediate_outputs
returns_list[pos] = f"{name}"
Copy link
Member Author

@huangjiyi huangjiyi Jun 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

上面的问题是因为 eager_gen.py 在解析返回值列表的代码中,没有考虑到 intermediate 类型的 output 不在 outputs List 最后的情况,已修复

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

将 index 的 intermediate 移除后,这段改动还需要吗?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

将 index 的 intermediate 移除后,这段改动还需要吗?

目前就先将indexintermediate 去除。这里的改动先保留,确实存在逻辑问题。

@huangjiyi
Copy link
Member Author

huangjiyi commented Jun 4, 2023

class MultiClassNMS2OpMaker : public MultiClassNMSOpMaker {
 public:
  void Make() override {
    MultiClassNMSOpMaker::Make();
    AddOutput("Index",
              "(phi::DenseTensor) A 2-D phi::DenseTensor with shape [No, 1] "
              "represents the "
              "index of selected bbox. The index is the absolute index cross "
              "batches.")
        .AsIntermediate();
  }
};

multiclass_nms3index 输出不能在 ops.yaml 里设置为 intermediate,因为设置了后在生成的 dygraph_function.h 中的返回值只有两个,而在单测 test/legacy_test/test_multiclass_nms_op.py 中需要 3 个返回值:

output, index, nms_rois_num = _C_ops.multiclass_nms3(
    bboxes, scores, rois_num, *attrs
)
然后跑单测会出现下面的错误:

test 1177
    Start 1177: test_multiclass_nms_op

1177: Test command: /home/cmake-3.18.0-Linux-x86_64/bin/cmake "-E" "env" "PYTHONPATH=/Paddle/build/python" "/usr/bin/python3.7" "/Paddle/tools/test_runner.py" "test_multiclass_nms_op"
1177: Test timeout computed to be: 120
1177: grep: warning: GREP_OPTIONS is deprecated; please use an alias or script
1177: I0604 12:25:29.903067 248644 interpretercore.cc:237] New Executor is Running.
1177: test_multiclass_nms_op failed
1177:  .....EssssE....
1177: ======================================================================
1177: ERROR: test_check_output (test_multiclass_nms_op.TestMulticlassNMS3Op)
1177: ----------------------------------------------------------------------
1177: Traceback (most recent call last):
1177:   File "/Paddle/build/test/legacy_test/test_multiclass_nms_op.py", line 790, in test_check_output
1177:     self.check_output_with_place(place)
1177:   File "/Paddle/build/test/legacy_test/eager_op_test.py", line 2042, in check_output_with_place
1177:     dygraph_checker.check()
1177:   File "/Paddle/build/test/legacy_test/eager_op_test.py", line 1806, in check
1177:     self.calculate_output()
1177:   File "/Paddle/build/test/legacy_test/eager_op_test.py", line 1878, in calculate_output
1177:     dygraph_outs = self.op_test._calc_python_api_output(place)
1177:   File "/Paddle/build/test/legacy_test/eager_op_test.py", line 1078, in _calc_python_api_output
1177:     return cal_python_api(self.python_api, args, kernel_sig)
1177:   File "/Paddle/build/test/legacy_test/eager_op_test.py", line 1015, in cal_python_api
1177:     ret_tuple = python_api(*args)
1177:   File "/Paddle/build/test/legacy_test/test_multiclass_nms_op.py", line 56, in multiclass_nms3
1177:     bboxes, scores, rois_num, *attrs
1177: ValueError: not enough values to unpack (expected 3, got 2)

@huangjiyi
Copy link
Member Author

@heavyrain-lzy,帮忙 Review 一下~

Copy link
Contributor

@heavyrain-lzy heavyrain-lzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@huangjiyi
Copy link
Member Author

@luotao1,帮忙豁免一下 CI ~

@luotao1 luotao1 merged commit 587f66e into PaddlePaddle:develop Jun 5, 2023
@huangjiyi huangjiyi deleted the autogen_multiclass_nms3 branch January 9, 2024 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contributor External developers HappyOpenSource 快乐开源活动issue与PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants