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

Refine default hooks and custom hooks priority rank. #1120

Merged
merged 3 commits into from
Jun 25, 2021

Conversation

mzr1996
Copy link
Member

@mzr1996 mzr1996 commented Jun 21, 2021

Motivation

Refine default hooks and custom hooks priority rank, and make it be convenient to users.

Modification

Use string to indicate the priority of default hooks, and add more priority ranks.

BC-breaking (Optional)

This PR changes the priority of default hooks of runner, if users already use a priority in their custom hook config, the execution order may change.

Use cases (Optional)

Users can insert a custom hook after optimizer hook by config

custom_hooks = [dict(type='CustomHook', priority='HIGH')]

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 tests 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.

@codecov
Copy link

codecov bot commented Jun 21, 2021

Codecov Report

Merging #1120 (d8a796e) into master (004c006) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1120      +/-   ##
==========================================
+ Coverage   67.62%   67.65%   +0.02%     
==========================================
  Files         159      159              
  Lines       10283    10297      +14     
  Branches     1857     1858       +1     
==========================================
+ Hits         6954     6966      +12     
- Misses       2964     2965       +1     
- Partials      365      366       +1     
Flag Coverage Δ
unittests 67.65% <100.00%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
mmcv/runner/base_runner.py 77.47% <100.00%> (ø)
mmcv/runner/priority.py 71.42% <100.00%> (+3.00%) ⬆️
mmcv/utils/misc.py 93.57% <0.00%> (-0.92%) ⬇️
mmcv/cnn/utils/weight_init.py 91.34% <0.00%> (-0.04%) ⬇️
mmcv/utils/__init__.py 88.23% <0.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 004c006...d8a796e. Read the comment docs.

`LOWER`.

And add unit tests for custom hook with the same priority as
default hooks.
@ZwwWayne ZwwWayne requested review from innerlee and ycxioooong June 24, 2021 14:35
@ZwwWayne
Copy link
Collaborator

LGTM, see if others have any comments.

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.

2 participants