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

Add RDN. #233

Merged
merged 9 commits into from
Apr 25, 2021
Merged

Add RDN. #233

merged 9 commits into from
Apr 25, 2021

Conversation

Yshuo-Li
Copy link
Collaborator

@Yshuo-Li Yshuo-Li commented Apr 1, 2021

Residual Dense Network for Image Super-Resolution.

@codecov
Copy link

codecov bot commented Apr 1, 2021

Codecov Report

Merging #233 (91a1354) into master (e374ebf) will increase coverage by 0.05%.
The diff coverage is 87.09%.

❗ Current head 91a1354 differs from pull request most recent head a784deb. Consider uploading reports for the commit a784deb to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #233      +/-   ##
==========================================
+ Coverage   79.93%   79.99%   +0.05%     
==========================================
  Files         159      160       +1     
  Lines        7941     8002      +61     
  Branches     1177     1185       +8     
==========================================
+ Hits         6348     6401      +53     
- Misses       1449     1456       +7     
- Partials      144      145       +1     
Flag Coverage Δ
unittests 79.99% <87.09%> (+0.05%) ⬆️

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

Impacted Files Coverage Δ
mmedit/models/backbones/sr_backbones/rdn.py 86.66% <86.66%> (ø)
mmedit/models/backbones/__init__.py 100.00% <100.00%> (ø)
mmedit/models/backbones/sr_backbones/__init__.py 100.00% <100.00%> (ø)

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 e374ebf...a784deb. Read the comment docs.

self.G0 = mid_channels
self.G = growth_rate
self.D = num_blocks
self.C = num_layers
Copy link
Collaborator

Choose a reason for hiding this comment

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

Such names (G0, G or D) are so ugly and confusing. Please rename it with regular terms.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In general, the name of variables should not contain the capital words.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, just use self.mid_channels = mid_channels

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

G, D, C e.t. are variables in the paper, I'll change them in code.

x (Tensor): Input tensor with shape (n, c, h, w).

Returns:
Tensor: Forward results, tensor with shape (n, c, h, w).
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the shape does not change after concatenating


Args:
in_channels (int): Channel number of inputs.
out_channels (int): Channel number of outputs.
Copy link
Contributor

Choose a reason for hiding this comment

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

docstring does not match __init__

def __init__(self, in_channels, growth_rate, num_layers):
super(RDB, self).__init__()
self.layers = nn.Sequential(*[
DenseLayer(in_channels + growth_rate * i, growth_rate)
Copy link
Contributor

Choose a reason for hiding this comment

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

is growth_rate an int or a float ratio?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is int, which is the channel number of dense layer output.

Paper: Residual Dense Network for Image Super-Resolution
Adapted from:
https://github.com/yulunzhang/RDN.git
https://github.com/yjn870/RDN-pytorch
Copy link
Contributor

Choose a reason for hiding this comment

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

Add licence info

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These GitHub projects don't have license files.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, need to contact them

Default: 4.
num_layer (int): Layer number in the Residual Dense Block.
Default: 8.
growth_rate(int): Channels growth in each layer of RDB.
Copy link
Contributor

Choose a reason for hiding this comment

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

rate, a terminology that is often related to percentage, is not good to describe this number.

How about just growth, or channel_growth or growth_num, etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

picked channel_growth.

@innerlee
Copy link
Contributor

Mostly some comments on naming. Also need to rebase master :)

@innerlee innerlee mentioned this pull request Apr 20, 2021
5 tasks
@innerlee
Copy link
Contributor

Conflict with master now


Paper: Residual Dense Network for Image Super-Resolution

Adapted from '[email protected]:yjn870/RDN-pytorch.git'
Copy link
Contributor

Choose a reason for hiding this comment

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

@innerlee
Copy link
Contributor

Linting fails

@Yshuo-Li Yshuo-Li requested a review from innerlee April 25, 2021 12:46
@innerlee innerlee merged commit 352ed6a into open-mmlab:master Apr 25, 2021
@Yshuo-Li Yshuo-Li deleted the rdn branch April 26, 2021 03:47
Yshuo-Li added a commit to Yshuo-Li/mmediting that referenced this pull request Jul 15, 2022
* add RDN

* Add docstring and test.

* Tiny fix.

* Tiny fix.

* Add license.

* Tiny Fix

* Tiny Fix

Co-authored-by: liyinshuo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants