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 receptive field search of CNN models. (TPAMI paper rf-next) #2056

Merged
merged 31 commits into from
Nov 22, 2022

Conversation

gasvn
Copy link
Contributor

@gasvn gasvn commented Jun 15, 2022

Motivation

Merging a general receptive field search method to mmcv and mmdet.
(paper: RF-Next: Efficient Receptive Field Search for Convolutional Neural Networks TPAMI 2022 pdf)

Modification

The RFSearch module is included.

BC-breaking (Optional)

Does the modification introduce changes that break the backward-compatibility of the downstream repositories?
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.

This pr is a part of pr in mmdet open-mmlab/mmdetection#8191

Checklist

Before PR:

  • I have read and followed the workflow indicated in the CONTRIBUTING.md to create this PR.
  • Pre-commit or linting tools indicated in CONTRIBUTING.md are used to fix the potential lint issues.
  • Bug fixes are covered by unit tests, the case that causes the bug should be added in the unit tests.
  • New functionalities are covered by complete unit tests. If not, please add more unit test to ensure the correctness.
  • The documentation has been modified accordingly, including docstring or example tutorials.

After PR:

  • If the modification has potential influence on downstream or other related projects, this PR should be tested with some of those projects, like MMDet or MMCls.
  • CLA has been signed and all committers have signed the CLA in this PR.

@CLAassistant
Copy link

CLAassistant commented Jun 15, 2022

CLA assistant check
All committers have signed the CLA.

@zhouzaida
Copy link
Member

Hi @gasvn , thanks for your contributions. Please add docstring and type hints for your added files.

@gasvn
Copy link
Contributor Author

gasvn commented Jun 16, 2022

Hi @gasvn , thanks for your contributions. Please add docstring and type hints for your added files.

I have added docstring and type hints.

@gasvn
Copy link
Contributor Author

gasvn commented Jul 1, 2022

Is there any further update about this pull request?


class ConvRFSearchOp(nn.Module):

def __init__(self, op_layer: nn.Module, global_config: Dict = {}):
Copy link

@jbwang1997 jbwang1997 Jul 1, 2022

Choose a reason for hiding this comment

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

Is better to set the default global_confg as None. The dictionary may cause some unexpected problems.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing the global_config to None cannot pass the code checking. So we keep it unchanged for now.

@gasvn
Copy link
Contributor Author

gasvn commented Aug 29, 2022

Is there anything I can do to make some progress? Thanks~

@zhouzaida
Copy link
Member

Is there anything I can do to make some progress? Thanks~

Hi @gasvn , sorry for my late reply. I'm thinking about where to put the implementation of this hook.

mmcv/cnn/rfsearch/search.py Outdated Show resolved Hide resolved
mmcv/cnn/rfsearch/search.py Outdated Show resolved Hide resolved
mmcv/cnn/rfsearch/search.py Show resolved Hide resolved
mmcv/cnn/rfsearch/search.py Outdated Show resolved Hide resolved
mmcv/cnn/rfsearch/search.py Outdated Show resolved Hide resolved
mmcv/cnn/rfsearch/search.py Outdated Show resolved Hide resolved
mmcv/cnn/rfsearch/search.py Outdated Show resolved Hide resolved
mmcv/cnn/rfsearch/operator.py Outdated Show resolved Hide resolved
mmcv/cnn/rfsearch/operator.py Outdated Show resolved Hide resolved
mmcv/cnn/rfsearch/operator.py Outdated Show resolved Hide resolved
@gasvn
Copy link
Contributor Author

gasvn commented Oct 3, 2022

Hi, I have solved the comments above. Please let me know if there is anything I can do. Thanks.

@zhouzaida
Copy link
Member

Hi, I have solved the comments above. Please let me know if there is anything I can do. Thanks.

Hi @gasvn , sorry for my late reply.

@lzyhha
Copy link

lzyhha commented Nov 21, 2022

Hello, it seems that there is an error in build_cu102 (Error: retrieving gpg key timed out.). Is it a package that failed to download? @zhouzaida

@lzyhha
Copy link

lzyhha commented Nov 21, 2022

I have tested this latest commit with RF-mmdetection, and everything is OK. And the RF-mmdetection has been updated following the changes in this latest commit.

And what is the schedule to merge this PR?

@zhouzaida

@zhouzaida
Copy link
Member

I re-run the job and it should be OK.

mmcv/cnn/__init__.py Outdated Show resolved Hide resolved
mmcv/cnn/__init__.py Outdated Show resolved Hide resolved
@zhouzaida
Copy link
Member

I have tested this latest commit with RF-mmdetection, and everything is OK. And the RF-mmdetection has been updated following the changes in this latest commit.

And what is the schedule to merge this PR?

@zhouzaida

Hi @lzyhha , this PR looks good to me.

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>
@lzyhha
Copy link

lzyhha commented Nov 22, 2022

Hello, thanks for your working. May I know when will this PR be completed and merged? @zhouzaida

Copy link
Contributor

@zytx121 zytx121 left a comment

Choose a reason for hiding this comment

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

LGTM

Co-authored-by: Yue Zhou <592267829@qq.com>
@zhouzaida zhouzaida added this to TO PICK in MMCV 2.x Nov 22, 2022
lzyhha and others added 2 commits November 22, 2022 14:56
Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>
@zhouzaida
Copy link
Member

Thanks for @gasvn and @lzyhha contributions. This PR will be merged after CI passes.

@gasvn
Copy link
Contributor Author

gasvn commented Nov 22, 2022

Thanks for @gasvn and @lzyhha contributions. This PR will be merged after CI passes.

Thanks everyone for your help!!!!!!

@zhouzaida zhouzaida merged commit 6e3827d into open-mmlab:master Nov 22, 2022
@lzyhha lzyhha mentioned this pull request Dec 15, 2022
7 tasks
@lzyhha
Copy link

lzyhha commented Dec 15, 2022

I create a new PR to support RF-Next in more applications, e.g., mmsegmentation. @zhouzaida

ckirchhoff2021 pushed a commit to ckirchhoff2021/mmcv that referenced this pull request Dec 21, 2022
* support rfsearch

* add labs for rfsearch

* format

* format

* add docstring and type hints

* clean code

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* rm unused func

* update code

* update code

* update code

* update  details

* fix details

* support asymmetric kernel

* support asymmetric kernel

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* add unit tests for rfsearch

* set device for Conv2dRFSearchOp

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* remove unused function search_estimate_only

* move unit tests

* Update tests/test_cnn/test_rfsearch/test_operator.py

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Update mmcv/cnn/rfsearch/operator.py

Co-authored-by: Yue Zhou <592267829@qq.com>

* change logger

* Update mmcv/cnn/rfsearch/operator.py

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>
Co-authored-by: lzyhha <819814373@qq.com>
Co-authored-by: Zhongyu Li <44114862+lzyhha@users.noreply.github.com>
Co-authored-by: Yue Zhou <592267829@qq.com>
@zhouzaida zhouzaida moved this from TO PICK to PICKED in MMCV 2.x Mar 20, 2023
zhouzaida added a commit to zhouzaida/mmcv that referenced this pull request Mar 20, 2023
* support rfsearch

* add labs for rfsearch

* format

* format

* add docstring and type hints

* clean code

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* rm unused func

* update code

* update code

* update code

* update  details

* fix details

* support asymmetric kernel

* support asymmetric kernel

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* add unit tests for rfsearch

* set device for Conv2dRFSearchOp

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* remove unused function search_estimate_only

* move unit tests

* Update tests/test_cnn/test_rfsearch/test_operator.py

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Update mmcv/cnn/rfsearch/operator.py

Co-authored-by: Yue Zhou <592267829@qq.com>

* change logger

* Update mmcv/cnn/rfsearch/operator.py

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>
Co-authored-by: lzyhha <819814373@qq.com>
Co-authored-by: Zhongyu Li <44114862+lzyhha@users.noreply.github.com>
Co-authored-by: Yue Zhou <592267829@qq.com>
zhouzaida pushed a commit to zhouzaida/mmcv that referenced this pull request Mar 20, 2023
* support rfsearch

* add labs for rfsearch

* format

* format

* add docstring and type hints

* clean code

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* rm unused func

* update code

* update code

* update code

* update  details

* fix details

* support asymmetric kernel

* support asymmetric kernel

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* add unit tests for rfsearch

* set device for Conv2dRFSearchOp

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* remove unused function search_estimate_only

* move unit tests

* Update tests/test_cnn/test_rfsearch/test_operator.py

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Update mmcv/cnn/rfsearch/operator.py

Co-authored-by: Yue Zhou <592267829@qq.com>

* change logger

* Update mmcv/cnn/rfsearch/operator.py

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>
Co-authored-by: lzyhha <819814373@qq.com>
Co-authored-by: Zhongyu Li <44114862+lzyhha@users.noreply.github.com>
Co-authored-by: Yue Zhou <592267829@qq.com>

[Fix] Fix skip_layer for RF-Next (open-mmlab#2489)

* judge skip_layer by fullname

* lint

* skip_layer first

* update unit test
zhouzaida pushed a commit that referenced this pull request Mar 20, 2023
* support rfsearch

* add labs for rfsearch

* format

* format

* add docstring and type hints

* clean code

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* rm unused func

* update code

* update code

* update code

* update  details

* fix details

* support asymmetric kernel

* support asymmetric kernel

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* add unit tests for rfsearch

* set device for Conv2dRFSearchOp

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* remove unused function search_estimate_only

* move unit tests

* Update tests/test_cnn/test_rfsearch/test_operator.py

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Update mmcv/cnn/rfsearch/operator.py

Co-authored-by: Yue Zhou <592267829@qq.com>

* change logger

* Update mmcv/cnn/rfsearch/operator.py

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>
Co-authored-by: lzyhha <819814373@qq.com>
Co-authored-by: Zhongyu Li <44114862+lzyhha@users.noreply.github.com>
Co-authored-by: Yue Zhou <592267829@qq.com>

[Fix] Fix skip_layer for RF-Next (#2489)

* judge skip_layer by fullname

* lint

* skip_layer first

* update unit test
tyomj pushed a commit to tyomj/mmcv that referenced this pull request May 8, 2023
* support rfsearch

* add labs for rfsearch

* format

* format

* add docstring and type hints

* clean code

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* rm unused func

* update code

* update code

* update code

* update  details

* fix details

* support asymmetric kernel

* support asymmetric kernel

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* Apply suggestions from code review

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

* add unit tests for rfsearch

* set device for Conv2dRFSearchOp

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* remove unused function search_estimate_only

* move unit tests

* Update tests/test_cnn/test_rfsearch/test_operator.py

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Apply suggestions from code review

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

* Update mmcv/cnn/rfsearch/operator.py

Co-authored-by: Yue Zhou <592267829@qq.com>

* change logger

* Update mmcv/cnn/rfsearch/operator.py

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>

Co-authored-by: Zaida Zhou <58739961+zhouzaida@users.noreply.github.com>
Co-authored-by: lzyhha <819814373@qq.com>
Co-authored-by: Zhongyu Li <44114862+lzyhha@users.noreply.github.com>
Co-authored-by: Yue Zhou <592267829@qq.com>

[Fix] Fix skip_layer for RF-Next (open-mmlab#2489)

* judge skip_layer by fullname

* lint

* skip_layer first

* update unit test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
MMCV 2.x
PICKED
Development

Successfully merging this pull request may close these issues.

None yet

7 participants