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

[Feature] Support PIDNet #2609

Merged
merged 18 commits into from
Mar 15, 2023
Merged

[Feature] Support PIDNet #2609

merged 18 commits into from
Mar 15, 2023

Conversation

xiexinch
Copy link
Collaborator

@xiexinch xiexinch commented Feb 16, 2023

Motivation

Support SOTA real-time semantic segmentation method in Paper with code

Paper: https://arxiv.org/pdf/2206.02066.pdf
Official repo: https://github.com/XuJiacong/PIDNet

Current results

Cityscapes

Model Ref mIoU mIoU (ours)
PIDNet-S 78.8 78.74
PIDNet-M 79.9 80.22
PIDNet-L 80.9 80.89

TODO

  • Support inference with official weights
  • Support training on Cityscapes
  • Update docstring
  • Add unit test

@xiexinch xiexinch added 1.x Related issue of 1.x version WIP Work in process labels Feb 16, 2023
@codecov
Copy link

codecov bot commented Feb 16, 2023

Codecov Report

Patch coverage: 79.43% and project coverage change: -0.02 ⚠️

Comparison is base (eca61d3) 83.44% compared to head (8c95ffc) 83.43%.

❗ Current head 8c95ffc differs from pull request most recent head 8379188. Consider uploading reports for the commit 8379188 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           dev-1.x    #2609      +/-   ##
===========================================
- Coverage    83.44%   83.43%   -0.02%     
===========================================
  Files          147      153       +6     
  Lines         8620     9054     +434     
  Branches      1293     1343      +50     
===========================================
+ Hits          7193     7554     +361     
- Misses        1205     1263      +58     
- Partials       222      237      +15     
Flag Coverage Δ
unittests 83.43% <79.43%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
mmseg/datasets/dataset_wrappers.py 49.18% <0.00%> (ø)
mmseg/datasets/transforms/transforms.py 91.05% <0.00%> (+1.55%) ⬆️
mmseg/models/losses/ohem_cross_entropy_loss.py 48.57% <48.57%> (ø)
mmseg/models/losses/boundary_loss.py 50.00% <50.00%> (ø)
mmseg/models/decode_heads/pid_head.py 60.56% <60.56%> (ø)
mmseg/datasets/transforms/formatting.py 93.18% <66.66%> (+3.43%) ⬆️
mmseg/apis/mmseg_inferencer.py 74.38% <71.66%> (+0.02%) ⬆️
mmseg/evaluation/metrics/citys_metric.py 86.66% <84.21%> (-3.34%) ⬇️
mmseg/models/utils/basic_block.py 91.48% <91.48%> (ø)
mmseg/models/backbones/pidnet.py 93.95% <93.95%> (ø)
... and 10 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@xiexinch xiexinch changed the title [WIP][Feature] Support PIDNet [Feature] Support PIDNet Feb 28, 2023
@xiexinch xiexinch removed the WIP Work in process label Feb 28, 2023
@MeowZheng MeowZheng self-requested a review March 2, 2023 08:20
configs/pidnet/README.md Outdated Show resolved Hide resolved

<a href="https://github.com/XuJiacong/PIDNet">Official Repo</a>

<a href="https://github.com/open-mmlab/mmdetection/blob/dev-3.x/mmdet/models/backbones/pidnet.py">Code Snippet</a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

this link should be checked.

ppm_channels=112,
num_stem_blocks=3,
num_branch_blocks=4,
init_cfg=dict(checkpoint='pretrain/PIDNet_L_ImageNet.pth')),
Copy link
Collaborator

Choose a reason for hiding this comment

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

how can users get the pretrained? Please add some notes in README

Comment on lines 58 to 59
def forward(self, x_p: Tensor, x_i: Tensor) -> Tensor:
if self.after_relu:
Copy link
Collaborator

Choose a reason for hiding this comment

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

please add docstring

mmseg/models/losses/boundary_loss.py Show resolved Hide resolved
Comment on lines 113 to 121
"""Forward function.
Args:
inputs (Tensor | tuple[Tensor]): Input tensor or tuple of
Tensor. When training, the input is a tuple of three tensors,
(p_feat, i_feat, d_feat), and the output is a tuple of three
tensors, (p_seg_logit, i_seg_logit, d_seg_logit).
When inference, only the head of integral branch is used, and
input is a tensor of integral feature map, and the output is
the segmentation logit.
Copy link
Collaborator

Choose a reason for hiding this comment

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

please format the docstring

Comment on lines 50 to 57
"""Forward function.
Args:
x (Tensor): Input tensor.
cls_seg (nn.Module, optional): The classification head.

Returns:
Tensor: Output tensor.
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

should follow the google docstring style

Comment on lines 94 to 96
self.i_head = BasePIDHead(in_channels, channels)
self.p_head = BasePIDHead(in_channels // 2, channels)
self.d_head = BasePIDHead(in_channels // 2, in_channels // 4)
Copy link
Collaborator

Choose a reason for hiding this comment

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

does i_head, p_head, and d_head use BN rather than SyncBN

Comment on lines 309 to 317
"""Make stem layer.
Args:
in_channels (int): Number of input channels.
channels (int): Number of output channels.
num_blocks (int): Number of blocks.

Returns:
Stem layer (nn.Sequential)
"""
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it didn't follow the google style.

configs/pidnet/README.md Show resolved Hide resolved
configs/pidnet/README.md Show resolved Hide resolved
Comment on lines 285 to 334
if num_stem_blocks == 2:
self.d_branch_layers = nn.ModuleList([
self._make_single_layer(BasicBlock, channels * 2, channels),
self._make_layer(Bottleneck, channels, channels, 1)
])
self.diff_1 = ConvModule(
channels * 4,
channels,
kernel_size=3,
padding=1,
bias=False,
norm_cfg=norm_cfg,
act_cfg=None)
self.diff_2 = ConvModule(
channels * 8,
channels * 2,
kernel_size=3,
padding=1,
bias=False,
norm_cfg=norm_cfg,
act_cfg=None)
self.spp = PAPPM(
channels * 16, ppm_channels, channels * 4, num_scales=5)
self.dfm = LightBag(channels * 4, channels * 4, norm_cfg=norm_cfg)
else:
self.d_branch_layers = nn.ModuleList([
self._make_single_layer(BasicBlock, channels * 2,
channels * 2),
self._make_single_layer(BasicBlock, channels * 2, channels * 2)
])
self.diff_1 = ConvModule(
channels * 4,
channels * 2,
kernel_size=3,
padding=1,
norm_cfg=norm_cfg,
act_cfg=None)
self.diff_2 = ConvModule(
channels * 8,
channels * 2,
kernel_size=3,
padding=1,
norm_cfg=norm_cfg,
act_cfg=None)
self.spp = DAPPM(
channels * 16, ppm_channels, channels * 4, num_scales=5)
self.dfm = Bag(
channels * 4, channels * 4, norm_cfg=norm_cfg, act_cfg=act_cfg)
self.d_branch_layers.append(
self._make_layer(Bottleneck, channels * 2, channels * 2, 1))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if num_stem_blocks == 2:
self.d_branch_layers = nn.ModuleList([
self._make_single_layer(BasicBlock, channels * 2, channels),
self._make_layer(Bottleneck, channels, channels, 1)
])
self.diff_1 = ConvModule(
channels * 4,
channels,
kernel_size=3,
padding=1,
bias=False,
norm_cfg=norm_cfg,
act_cfg=None)
self.diff_2 = ConvModule(
channels * 8,
channels * 2,
kernel_size=3,
padding=1,
bias=False,
norm_cfg=norm_cfg,
act_cfg=None)
self.spp = PAPPM(
channels * 16, ppm_channels, channels * 4, num_scales=5)
self.dfm = LightBag(channels * 4, channels * 4, norm_cfg=norm_cfg)
else:
self.d_branch_layers = nn.ModuleList([
self._make_single_layer(BasicBlock, channels * 2,
channels * 2),
self._make_single_layer(BasicBlock, channels * 2, channels * 2)
])
self.diff_1 = ConvModule(
channels * 4,
channels * 2,
kernel_size=3,
padding=1,
norm_cfg=norm_cfg,
act_cfg=None)
self.diff_2 = ConvModule(
channels * 8,
channels * 2,
kernel_size=3,
padding=1,
norm_cfg=norm_cfg,
act_cfg=None)
self.spp = DAPPM(
channels * 16, ppm_channels, channels * 4, num_scales=5)
self.dfm = Bag(
channels * 4, channels * 4, norm_cfg=norm_cfg, act_cfg=act_cfg)
self.d_branch_layers.append(
self._make_layer(Bottleneck, channels * 2, channels * 2, 1))
if num_stem_blocks == 2:
self.d_branch_layers = ...
channel_expand = 1
bag_module = LightBag
else:
self.d_branch_layers = ...
channel_expend = 2
bag_module = Bag
self.diff_1 = ConvModule(
channels * 4,
channels * channel_expand,
kernel_size=3,
padding=1,
bias=False,
norm_cfg=norm_cfg,
act_cfg=None)
self.diff_2 = ConvModule(
channels * 8,
channels * 2,
kernel_size=3,
padding=1,
bias=False,
norm_cfg=norm_cfg,
act_cfg=None)
self.spp = PAPPM(
channels * 16, ppm_channels, channels * 4, num_scales=5)
self.dfm = bag_module(channels * 4, channels * 4, norm_cfg=norm_cfg)

mmseg/models/backbones/pidnet.py Show resolved Hide resolved
from mmseg.registry import MODELS


@MODELS.register_module()
Copy link
Collaborator

Choose a reason for hiding this comment

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

it should be added docstring.

mmseg/models/utils/basic_block.py Show resolved Hide resolved
self.conv_cfg = conv_cfg

self.scales = ModuleList()
for i in range(num_scales):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would be better that this loop is from 1 to num_scales-1 and move scales[0] definition and scales[num_scales-1] out of the loop. Too much if else will hurt to read and efficiency, as every iteration of the loop must do if else.

<!-- [IMAGE] -->

<div align=center>
<img src="https://raw.githubusercontent.com/XuJiacong/PIDNet/main/figs/pidnet.jpg" width="400"/>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This image is a little small when preview README


| Method | Backbone | Crop Size | Lr schd | Mem (GB) | Inf time (fps) | mIoU | mIoU(ms+flip) | config | download |
| ------ | -------- | --------- | ------- | -------- | -------------- | ----- | ------------- | ----------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| PIDNet | PIDNet-S | 1024x1024 | 120000 | 3466 | 80.82 | 78.74 | 80.87 | [config](https://github.com/open-mmlab/mmsegmentation/blob/dev-1.x/configs/pidnet/pidnet-s_2xb6-120k_1024x1024-cityscapes.py) | [model](https://download.openmmlab.com/mmsegmentation/v0.5/pidnet/pidnet-s_2xb6-120k_1024x1024-cityscapes/pidnet-s_2xb6-120k_1024x1024-cityscapes_20230302_191700-bb8e3bcc.pth) \| [log](https://download.openmmlab.com/mmsegmentation/v0.5/pidnet/pidnet-s_2xb6-120k_1024x1024-cityscapes/pidnet-s_2xb6-120k_1024x1024-cityscapes_20230302_191700.json) |
Copy link
Collaborator

Choose a reason for hiding this comment

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

3466/1024 GB

Comment on lines 35 to 40
Args:
bd_pre (Tensor): Predictions of the boundary head.
bd_gt (Tensor): Ground truth of the boundary.

Returns:
Tensor: Loss tensor.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Args:
bd_pre (Tensor): Predictions of the boundary head.
bd_gt (Tensor): Ground truth of the boundary.
Returns:
Tensor: Loss tensor.
Args:
bd_pre (Tensor): Predictions of the boundary head.
bd_gt (Tensor): Ground truth of the boundary.
Returns:
Tensor: Loss tensor.

Comment on lines 54 to 59
Args:
score (Tensor): Predictions of the segmentation head.
target (Tensor): Ground truth of the image.

Returns:
Tensor: Loss tensor.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Args:
score (Tensor): Predictions of the segmentation head.
target (Tensor): Ground truth of the image.
Returns:
Tensor: Loss tensor.
Args:
score (Tensor): Predictions of the segmentation head.
target (Tensor): Ground truth of the image.
Returns:
Tensor: Loss tensor.

Copy link
Collaborator

@MeowZheng MeowZheng left a comment

Choose a reason for hiding this comment

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

please add PIDNet in model zoo of readme

@MeowZheng MeowZheng merged commit dd47cef into open-mmlab:dev-1.x Mar 15, 2023
nahidnazifi87 pushed a commit to nahidnazifi87/mmsegmentation_playground that referenced this pull request Apr 5, 2024
## Motivation

Support SOTA real-time semantic segmentation method in [Paper with
code](https://paperswithcode.com/task/real-time-semantic-segmentation)

Paper: https://arxiv.org/pdf/2206.02066.pdf
Official repo: https://github.com/XuJiacong/PIDNet

## Current results

**Cityscapes**

|Model|Ref mIoU|mIoU (ours)|
|---|---|---|
|PIDNet-S|78.8|78.74|
|PIDNet-M|79.9|80.22|
|PIDNet-L|80.9|80.89|

## TODO

- [x] Support inference with official weights
- [x] Support training on Cityscapes
- [x] Update docstring
- [x] Add unit test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x Related issue of 1.x version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants