【Hackathon + No.150】PaddleRS集成PaddleDetection的旋转框检测能力 #100

Asthestarsfalll · 2023-03-01T07:14:35Z

PR types

New features

PR changes

Others

Description

希望可以讨论一下集成的方式

Asthestarsfalll · 2023-03-03T10:36:00Z

想问下最终期望的效果以及验收标准是什么呢，需要在对应的数据集上进行训练或推理吗？

Bobholamovic

想问下最终期望的效果以及验收标准是什么呢，需要在对应的数据集上进行训练或推理吗？

我们的目标是：为目标检测任务新增旋转框检测能力支持，用户可以像PaddleRS其它任务一样，通过简单的API调用使用这一能力。

验收标准如下：

核心要求：增加PaddleDetection已支持的至少1个旋转框检测模型，提供一个DOTA数据集上训练的tutorial。
文档：更新API文档，增加相关说明；按照接入新模型要求，更新PaddleRS已支持模型列表等。
测试：根据PaddleRS开发指南，为加入的每个模型编写单测。另外需添加TIPC基础训推链条。

我review了一下，我觉得整体思路清晰，代码实现已经满足了上述的核心要求。有几个点想讨论一下：

要求用户自行构造 batch transform、并将其像 transform 一样作为参数传入训练器是一个很好的想法。之前的目标检测任务训练器太硬编码了，我们是不是可以整体做一个重构，让所有目标检测训练器都支持batch_transform输入。
当前目标检测训练器实现过于复杂，加入旋转框检测功能之后更加复杂，如果可以的话，希望在添加旋转框功能的同时也可以在代码层面做一些简化和优化，去冗余、降耦合。

Asthestarsfalll · 2023-03-15T06:51:13Z

当前目标检测训练器实现过于复杂，加入旋转框检测功能之后更加复杂，如果可以的话，希望在添加旋转框功能的同时也可以在代码层面做一些简化和优化，去冗余、降耦合。

这方面没有太多的想法，目前的思路如下：

减少训练入参，将optimizer和lr scheduler剥离出来，使用时需要预先调用set_optimizer和set_lrsheduler方法，支持传入不同的优化器和学习率调度器，未调用则使用默认的optimizer和lr shceduler。
目标检测器支持使用不同的neck head loss等，而不是硬编码在__init__里

Bobholamovic · 2023-03-15T10:14:23Z

可以按照你的想法实现一下～我们的目标是：1. 用户能够简洁地使用API；2. 在满足1的情况下代码尽可能优雅。

Bobholamovic · 2023-04-19T14:17:22Z

好像有点儿bug，CI报错如下：

有时间的时候辛苦看看啦~

Bobholamovic

这个任务比较复杂，工作量也相当大。首先感谢贡献代码~ 对于这个PR，我有如下的意见和建议：

我注意到从paddledetection集成功能到paddlers/models/ppdet/modeling的过程中似乎做了一些手工改写，这不利于后续我们同步更新版本的paddledetection。建议使用这个脚本进行代码的自动迁移，而不手工改动models/ppdet中的内容。
上次我们讨论过对目标检测训练器的重构和优化问题，我担心目前所做的部分优化可能会影响接口易用性（对于PaddleRS来说，这比代码质量具有更高的优先级）。我留了一些comments，咱们可以再讨论一下。
建议完整跑一下模型的训练、验证、动态图预测、导出和静态图推理，另外也请保证tutorial可以跑通。从代码上来看，目前的实现很可能会有一些问题……
PaddleRS对代码风格（包括注释）设置了较强的规则，具体请参考开发指南。不过我们现在可以先专注改动内容，这一点也可以先delay。

其它意见、建议或讨论详见comments。

paddlers/datasets/coco.py

tutorials/train/rotated_object_detection/data/data/.gitignore

tutorials/train/rotated_object_detection/fcosr.py

Bobholamovic · 2023-04-20T06:11:07Z

tutorials/train/rotated_object_detection/fcosr.py

+# 定义训练和验证时使用的数据变换（数据增强、预处理等）
+# 使用Compose组合多种变换方式。Compose中包含的变换将按顺序串行执行
+# API说明：https://github.com/PaddlePaddle/PaddleRS/blob/develop/docs/apis/data.md
+train_transforms = T.Compose([


这里好像还是按照旧的tutorial样式，请参考目前develop分支其它的tutorial改写一下

tutorials/train/rotated_object_detection/fcosr.py

paddlers/tasks/object_detector.py

paddlers/datasets/coco.py

tutorials/train/rotated_object_detection/fcosr.py

Asthestarsfalll · 2023-04-20T07:01:38Z

3. 建议完整跑一下模型的训练、验证、动态图预测、导出和静态图推理，另外也请保证tutorial可以跑通。从代码上来看，目前的实现很可能会有一些问题……

是的，目前并没有跑代码，想等到确定好代码设计后再跑通

paddlers/tasks/object_detector.py

Asthestarsfalll · 2023-04-20T10:39:00Z

我注意到从paddledetection集成功能到paddlers/models/ppdet/modeling的过程中似乎做了一些手工改写，这不利于后续我们同步更新版本的paddledetection。建议使用这个脚本进行代码的自动迁移，而不手工改动models/ppdet中的内容。

使用该方法同步了ppdet，并且使用了pre-commit检查，但是格式和之前还是有很多不一致，导致修改文件增多

Asthestarsfalll · 2023-04-25T14:54:36Z

目前可以进行训练，但还未进行验证

Bobholamovic · 2023-04-26T02:13:51Z

目前可以进行训练，但还未进行验证

赞！不过从log看起来loss好像有些震荡，建议可以验证一下在小数据集上完整训练一次，精度是否正常~

Asthestarsfalll · 2023-05-01T06:45:20Z

截取了前600个样本进行训练，结果如下：

Asthestarsfalll · 2023-05-02T07:47:22Z

prepare_dota:

Bobholamovic

tql！

我觉得训练器的rotate参数的引入是符合PaddleRS风格的优秀设计~ 不过，对于batch transforms的组织，我有一些别的想法：从依赖关系上看，batch transforms属于对数据进行预处理的内容，其实放在dataset中实现更加合适。目前dataset预留了一个属性，而主要的逻辑还是在训练器中。你觉得，把batch transforms的主要逻辑放在dataset中（BaseDataset有一个collate_fn方法可以实现，其实这样做说不定还可以消除一些冗余代码），在使用时也和transforms一样通过dataset配置，这样会不会更合适、更优雅，实现上有没有怎么样的难点？

另外我对PR还有一些小疑问和讨论，详见评论~

docs/apis/data_cn.md

tools/prepare_dataset/common.py

paddlers/tasks/object_detector.py

Bobholamovic · 2023-05-04T02:58:15Z

paddlers/transforms/operators.py

@@ -1881,8 +1883,17 @@ def __init__(self, num_max_boxes=50):
        """

        self.num_max_boxes = num_max_boxes
+        self.return_gt_mask = return_gt_mask


这个属性加了之后是用在哪里哦？

paddlers/transforms/rotated_operators.py

Asthestarsfalll · 2023-05-04T04:26:15Z

从依赖关系上看，batch transforms属于对数据进行预处理的内容，其实放在dataset中实现更加合适。目前dataset预留了一个属性，而主要的逻辑还是在训练器中。你觉得，把batch transforms的主要逻辑放在dataset中（BaseDataset有一个collate_fn方法可以实现，其实这样做说不定还可以消除一些冗余代码），在使用时也和transforms一样通过dataset配置，这样会不会更合适、更优雅，实现上有没有怎么样的难点？

最初我的想法也是将batch_transform放在dataset中实现，但是考虑到不同的训练器似乎有不同的默认batch_transform方法，如果让用户手动传入似乎会加重使用时的心智负担，另外考虑到最初实现时就是这样，可能有什么考量，所以还是选择在训练器中修改了。
我认为可以将batch_transform作为dataset中的可选参数，同时仍然保留不同训练器中的默认batch_transform。

Bobholamovic · 2023-05-04T04:41:28Z

从依赖关系上看，batch transforms属于对数据进行预处理的内容，其实放在dataset中实现更加合适。目前dataset预留了一个属性，而主要的逻辑还是在训练器中。你觉得，把batch transforms的主要逻辑放在dataset中（BaseDataset有一个collate_fn方法可以实现，其实这样做说不定还可以消除一些冗余代码），在使用时也和transforms一样通过dataset配置，这样会不会更合适、更优雅，实现上有没有怎么样的难点？

最初我的想法也是将batch_transform放在dataset中实现，但是考虑到不同的训练器似乎有不同的默认batch_transform方法，如果让用户手动传入似乎会加重使用时的心智负担，另外考虑到最初实现时就是这样，可能有什么考量，所以还是选择在训练器中修改了。我认为可以将batch_transform作为dataset中的可选参数，同时仍然保留不同训练器中的默认batch_transform。

嗯嗯，我理解是不是用户可以通过dataset构造时的传参自定义batch transforms（就像在model.train中传入参数一样，这一操作非必须，如果不指定就不使用batch transforms），不过在用户未定义的场合，训练器可以使用自己默认的batch transforms（访问并检查dataset的batch_transforms这一公开属性本身是可依赖的行为，而在训练器中隐式修改dataset的属性虽然从Pythonic的角度来说不能算是优雅的做法，但PaddleRS目前确实很大程度上依赖于这种做法）。

咱们的思路是一致的不？

Bobholamovic · 2023-05-04T04:43:13Z

而在训练器中隐式修改dataset的属性虽然从Pythonic的角度来说不能算是优雅的做法，但PaddleRS目前确实很大程度上依赖于这种做法

对于这一点，或许也可以考虑给dataset.collate_fn添加一个可选的参数batch_transforms，以做临时的替换

Asthestarsfalll · 2023-05-04T05:05:51Z

咱们的思路是一致的不？

嗯嗯是的

Asthestarsfalll · 2023-05-04T08:17:04Z

@Bobholamovic 已更新～

Bobholamovic

赞！效率好高呀~

整体的思路应该可以定型了，dataset和object detector很符合我的构想和期望。有一些细节可能还需要完善一下，例如代码风格、docstring、文档等等（这次我提的comments只覆盖了一部分，后续可能还需要继续进行review）。

另外关于YOLOv3和FOCS这两个具体的模型，我有个疑问希望能讨论一下，详见评论区。

docs/data/dataset_cn.md

docs/data/dataset_en.md

docs/intro/data_prep_cn.md

docs/intro/data_prep_en.md

tutorials/train/object_detection/fcosr.py

paddlers/tasks/object_detector.py

tutorials/train/object_detection/fcosr.py

Bobholamovic

我提交了一个commit，做了一些文档润色和docstring格式上的修改。对于FCOSR和YOLOv3是否拆成两个类的问题还是想要再讨论一下（详见comment）。另外，paddlers/transforms/rbox_utils.py以及paddlers/transforms/rotated_operators.py这两个文件中的docstring好像不符合PaddleRS规范（而且后者没有包含版权信息），请检查修改一下。

docs/intro/model_zoo_cn.md

docs/intro/model_zoo_en.md

Bobholamovic · 2023-05-06T09:51:10Z

paddlers/transforms/rbox_utils.py

+    Return:
+        iou (Tensor): iou between box1 and box2 with the shape [N, M1, M2]
+    """
+    from ext_op import rbox_iou


这个ext_op是一个库嘛？

这个是ppdet里的库，需要手动安装一下

这个op是一定要支持的嘛？因为我们并没有在文档里告知用户需要手动安装这个库，也就是说按照标准流程安装的PaddleRS无法正常使用这个op，这样的话可能会让用户感到疑惑

如果可能的话，这一期可以暂时不支持这个op，后续我们统一处理需要手动安装的算子

ext_op在训练时用于计算loss，是一定要支持的，可以在tutorials里面说明一下，或者直接在setup.py里安装ext_op呢？

这样的话请在安装文档里增加相关说明

paddlers/tasks/object_detector.py

Bobholamovic

我提交了一个commit，做了一些文档润色和docstring格式上的修改。对于FCOSR和YOLOv3是否拆成两个类的问题还是想要再讨论一下（详见comment）。另外，paddlers/transforms/rbox_utils.py以及paddlers/transforms/rotated_operators.py这两个文件中的docstring好像不符合PaddleRS规范（而且后者没有包含版权信息），请检查修改一下。

Asthestarsfalll · 2023-05-08T05:13:00Z

已更新，辛苦晖哥看下～

Bobholamovic

在YOLOv3和FCOSR的接口及实现上我想我们已经达成共识，辛苦更新一下~ 另外我提了一个commit对docstring做了一些修复和润色。现在这个PR应该已经接近合入啦

Bobholamovic · 2023-05-08T06:12:03Z

paddlers/transforms/rbox_utils.py

+    Return:
+        iou (Tensor): iou between box1 and box2 with the shape [N, M1, M2]
+    """
+    from ext_op import rbox_iou


这个op是一定要支持的嘛？因为我们并没有在文档里告知用户需要手动安装这个库，也就是说按照标准流程安装的PaddleRS无法正常使用这个op，这样的话可能会让用户感到疑惑

Bobholamovic · 2023-05-08T06:12:31Z

paddlers/transforms/rbox_utils.py

+    Return:
+        iou (Tensor): iou between box1 and box2 with the shape [N, M1, M2]
+    """
+    from ext_op import rbox_iou


如果可能的话，这一期可以暂时不支持这个op，后续我们统一处理需要手动安装的算子

paddlers/tasks/object_detector.py

Bobholamovic

LGTM.

[Feat]: Support rotate object detection

304ee65

Asthestarsfalll mentioned this pull request Mar 1, 2023

【PaddlePaddle Hackathon 第四期】任务总览 PaddlePaddle/Paddle#50629

Closed

Bobholamovic self-requested a review March 1, 2023 08:26

Bobholamovic self-assigned this Mar 1, 2023

Bobholamovic reviewed Mar 8, 2023

View reviewed changes

Ligoml mentioned this pull request Mar 13, 2023

【PaddlePaddle Hackathon 第四期】任务总览 PaddlePaddle/Paddle#51281

Closed

Merge branch 'develop' into rotate

8fb6b72

Asthestarsfalll closed this Mar 22, 2023

update and simplify object_detection

943d639

Asthestarsfalll reopened this Apr 19, 2023

Merge branch 'develop' into rotate

83663a9

Bobholamovic self-requested a review April 19, 2023 13:29

fix import error

fd26f27

Bobholamovic requested changes Apr 20, 2023

View reviewed changes

update YOLOV3 for rotated object detection

bb2fd90

Asthestarsfalll commented Apr 20, 2023

View reviewed changes

paddlers/tasks/object_detector.py Show resolved Hide resolved

Asthestarsfalll added 2 commits April 20, 2023 18:19

Synchronize ppdet and handle some chores

076e51b

fix

ea5b7b6

Update for FCOSR and add some docs

1a3184f

Merge branch 'develop' into rotate

9912733

Asthestarsfalll added 2 commits May 2, 2023 15:52

update sth

b527b07

fix evaluate

bfc532b

Bobholamovic reviewed May 4, 2023

View reviewed changes

dataset support batch_transforms

4d6429b

Bobholamovic reviewed May 4, 2023

View reviewed changes

Asthestarsfalll and others added 5 commits May 5, 2023 10:22

fix ESNet

f0adfa6

fix preprate_dota

8b1a686

update according to comments

f520521

fix default argument missing

a6064d4

Polish docs and style

f293533

Bobholamovic requested changes May 6, 2023

View reviewed changes

Asthestarsfalll added 2 commits May 8, 2023 12:52

update docs and docstring

7aa5e27

format codestyle

06fa8f5

Fix and polish

c5a0f32

Bobholamovic reviewed May 8, 2023

View reviewed changes

add docs for ext_op

abdd079

Bobholamovic approved these changes May 8, 2023

View reviewed changes

Bobholamovic merged commit c6f15f7 into PaddlePaddle:develop May 8, 2023
4 of 9 checks passed

paddle-bot bot added the contributor label Mar 8, 2024

【Hackathon + No.150】PaddleRS集成PaddleDetection的旋转框检测能力 #100

【Hackathon + No.150】PaddleRS集成PaddleDetection的旋转框检测能力 #100

Conversation

Asthestarsfalll commented Mar 1, 2023 • edited

PR types

PR changes

Description

Asthestarsfalll commented Mar 3, 2023

Bobholamovic left a comment

Choose a reason for hiding this comment

Asthestarsfalll commented Mar 15, 2023

Bobholamovic commented Mar 15, 2023

Bobholamovic commented Apr 19, 2023

Bobholamovic left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Asthestarsfalll commented Apr 20, 2023

Asthestarsfalll commented Apr 20, 2023

Asthestarsfalll commented Apr 25, 2023

Bobholamovic commented Apr 26, 2023

Asthestarsfalll commented May 1, 2023

Asthestarsfalll commented May 2, 2023

Bobholamovic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Asthestarsfalll commented May 4, 2023

Bobholamovic commented May 4, 2023

Bobholamovic commented May 4, 2023

Asthestarsfalll commented May 4, 2023 • edited

Asthestarsfalll commented May 4, 2023

Bobholamovic left a comment • edited

Choose a reason for hiding this comment

Bobholamovic left a 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

Bobholamovic May 8, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bobholamovic left a comment

Choose a reason for hiding this comment

Asthestarsfalll commented May 8, 2023

Bobholamovic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Bobholamovic left a comment

Choose a reason for hiding this comment

Asthestarsfalll commented Mar 1, 2023 •

edited

Bobholamovic left a comment •

edited

Asthestarsfalll commented May 4, 2023 •

edited

Bobholamovic left a comment •

edited

Bobholamovic May 8, 2023 •

edited