[Fix] Restrict the warning message

[yyz561]

Thanks for your contribution and we appreciate it a lot. The following instructions would make your pull request more healthy and more easily get feedback. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.

Motivation

When I implemented a CenterNet2 model in mmdet, there was a situation when I had to use GroupNorm after Conv2d, and the Conv2d layer was supposed to have a bias. Once I finished the modeling part, I got a warning message 'ConvModule has norm and bias at the same time'. At first, I was not so sure about this warning, it made me uneased. Then I did a little research and realized that for BatchNorm (not so sure about SyncBN), the bias does not affect the calculation. But for other kinds of Norm, e.g., GN, I am pretty sure the bias does give some influence. I think maybe we should restrict the popping of this warning message, since it can be quite misleading for new starters.

Modification

Restrict a warning message of 'ConvModule has norm and bias at the same time' to BN, BN1, BN2 and BN3 normalization.

BC-breaking (Optional)

Does the modification introduce changes that break the backward-compatibility of the downstream repos?
If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.

Use cases (Optional)

If this PR introduces a new feature, it is better to list some use cases here, and update the documentation.

Checklist

1. Pre-commit or other linting tools are used to fix the potential lint issues.
2. The modification is covered by complete unit tests. If not, please add more unit test to ensure the correctness.
3. If the modification has potential influence on downstream projects, this PR should be tested with downstream projects, like MMDet or MMCls.
4. The documentation has been modified accordingly, like docstring or example tutorials.

[CLAassistant]

All committers have signed the CLA.

[zhouzaida]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

hi @yyz561 , please sign the CLA

[zhouzaida]

hi, please fix the CI

[yyz561]

hi, please fix the CI

Soryy, didn't noticed that code too long, will fix it immediately.

[yyz561]

hi, please fix the CI

It seems that the password is not valid for git push since yesterday. It took me some time to solve the problem, sorry for the delay. It should work now.

[zhouzaida]

hi, please fix the CI

It seems that the password is not valid for git push since yesterday. It took me some time to solve the problem, sorry for the delay. It should work now.

yet, thanks for your contribution

[zhouzaida]

we also need to add a unittest

[yyz561]

we also need to add a unittest

will this work? I didn't

we also need to add a unittest

Sorry mate, my first time to write a unit test, will this work?

[zhouzaida]

we also need to add a unittest

will this work? I didn't

we also need to add a unittest

Sorry mate, my first time to write a unit test, will this work?

I think it will work

[yyz561]

we also need to add a unittest

will this work? I didn't

we also need to add a unittest

Sorry mate, my first time to write a unit test, will this work?

I think it will work

I am exhausted by the format issue... both here and in another PR, could you please name a few external tools I can use? Thanks a lot.

[zhouzaida]

hi, you can refer to https://github.com/open-mmlab/mmcv/blob/master/CONTRIBUTING.md.

[codecov]

Codecov Report

Merging #1267 (76af0d0) into master (8aab4f2) will increase coverage by 0.05%.
The diff coverage is 93.10%.

❗ Current head 76af0d0 differs from pull request most recent head 817ea57. Consider uploading reports for the commit 817ea57 to get more accurate results

@@            Coverage Diff             @@
##           master    #1267      +/-   ##
==========================================
+ Coverage   68.23%   68.28%   +0.05%     
==========================================
  Files         160      161       +1     
  Lines       10722    10744      +22     
  Branches     1969     1973       +4     
==========================================
+ Hits         7316     7337      +21     
- Misses       3022     3023       +1     
  Partials      384      384              

Flag	Coverage Δ
unittests	68.28% <93.10%> (+0.05%)	⬆️

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

Impacted Files	Coverage Δ
mmcv/runner/default_constructor.py	77.77% <77.77%> (ø)
mmcv/cnn/bricks/conv_module.py	100.00% <100.00%> (ø)
mmcv/runner/__init__.py	100.00% <100.00%> (ø)
mmcv/runner/builder.py	100.00% <100.00%> (ø)
mmcv/runner/hooks/logger/pavi.py	71.42% <100.00%> (+0.41%)	⬆️
mmcv/version.py	82.35% <100.00%> (ø)
mmcv/video/processing.py	77.41% <100.00%> (+0.37%)	⬆️
mmcv/cnn/utils/weight_init.py	85.92% <0.00%> (+0.36%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8aab4f2...817ea57. Read the comment docs.

[zhouzaida]

LGTM

[zhouzaida]

hi @yyz561 , after reading the GN formula, I found the bias of conv also has no effect for the output of GN

[yyz561]

hi @yyz561 , after reading the GN formula, I found the bias of conv also has no effect for the output of GN

Hey, I didn't read the actual coding, but it is easy to do a GN experience. Try the following code:

import torch 
import torch.nn as nn
torch.manual_seed(0)
input = torch.randn(1, 4, 4, 4)
conv = nn.Conv2d(4, 4, 3, padding=1, bias=True)
norm = nn.GroupNorm(2, 4)
output = norm(conv(input))
print(output[0, 0, :, :].view(-1))

for comparison:

import torch 
import torch.nn as nn
torch.manual_seed(0)
input = torch.randn(1, 4, 4, 4)
conv = nn.Conv2d(4, 4, 3, padding=1, bias=False)
norm = nn.GroupNorm(2, 4)
output = norm(conv(input))
print(output[0, 0, :, :].view(-1))

[yyz561]

Hey @zhouzaida, I wrote an blog to talk about the problem. At the same time, I realized that bias of conv also has no effect for the output of Instance Norm too. Could you please have a look at the blog? https://zhuanlan.zhihu.com/p/403444336

[zhouzaida]

Hey @zhouzaida, I wrote an blog to talk about the problem. At the same time, I realized that bias of conv also has no effect for the output of Instance Norm too. Could you please have a look at the blog? https://zhuanlan.zhihu.com/p/403444336

yet, you are right. Thanks for your patience

[yyz561]

@zhouzaida do you agree with me to change the warning message into: "Unnecessary conv bias before batch/instance norm"

[zhouzaida]

@zhouzaida do you agree with me to change the warning message into: "Unnecessary conv bias before batch/instance norm"

Agree

[yyz561]

@zhouzaida do you agree with me to change the warning message into: "Unnecessary conv bias before batch/instance norm"

Agree

"First-time contributors need a maintainer to approve running workflows."

Thank you very much. Although it is a trivial change, I learned a lot through the process.