Skip to content

[Refactor] refactor EvalHook#395

Merged
innerlee merged 16 commits intoopen-mmlab:masterfrom
dreamerlin:polish_eval_hook
Jan 27, 2021
Merged

[Refactor] refactor EvalHook#395
innerlee merged 16 commits intoopen-mmlab:masterfrom
dreamerlin:polish_eval_hook

Conversation

@dreamerlin
Copy link
Copy Markdown
Collaborator

@dreamerlin dreamerlin commented Nov 28, 2020 •
edited
Loading

OverView

This PR refactors EvalHook

  1. Add by_epoch param to determine perform evaluation by epoch or by iteration.
  2. Add automatically assign a compare key when users do not set one.
  3. Save checkpoint by creating a symlink as well as record best info in runner hook meta
  4. Refactor the code structure

Migration

Configs files need to be changed

  1. Rename EpochEvalHook to EvalHook, DistEpochEvalHook to DistEvalHook.
  2. Rename key_indicator to save_best.

For example, in slowfast_kinetics_pretrained_r50_4x16x1_20e_ava_rgb.py cfg file

from

evaluation = dict(interval=1, key_indicator='mAP@0.5IOU')

to

evaluation = dict(interval=1, save_best='mAP@0.5IOU')

Misc.

This Part may further be merged into MMCV or applied to other MM codebases.

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 28, 2020 •
edited
Loading

Codecov Report

Merging #395 (cbfbf82) into master (b747208) will increase coverage by 0.07%.
The diff coverage is 85.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #395      +/-   ##
==========================================
+ Coverage   84.76%   84.84%   +0.07%     
==========================================
  Files         121      121              
  Lines        8579     8604      +25     
  Branches     1416     1427      +11     
==========================================
+ Hits         7272     7300      +28     
+ Misses        953      947       -6     
- Partials      354      357       +3
Flag Coverage Δ
unittests 84.83% <85.14%> (+0.07%) ⬆️

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

Impacted Files Coverage Δ
mmaction/apis/train.py 13.63% <50.00%> (ø)
mmaction/core/evaluation/eval_hooks.py 88.11% <84.44%> (+4.64%) ⬆️
mmaction/core/evaluation/__init__.py 100.00% <100.00%> (ø)
mmaction/datasets/activitynet_dataset.py 92.79% <100.00%> (+0.06%) ⬆️
mmaction/datasets/base.py 60.71% <100.00%> (ø)
mmaction/datasets/hvu_dataset.py 96.10% <100.00%> (+0.05%) ⬆️
mmaction/datasets/ssn_dataset.py 92.72% <100.00%> (+0.02%) ⬆️

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 b747208...45c85e9. Read the comment docs.

@dreamerlin dreamerlin changed the title [Improvment] Polish eval hook [Refactor] Polish eval hook Dec 16, 2020
@dreamerlin dreamerlin force-pushed the polish_eval_hook branch 2 times, most recently from 05cd173 to ad039ab Compare December 19, 2020 09:19
@dreamerlin dreamerlin mentioned this pull request Dec 19, 2020
Closed
4 tasks
@dreamerlin dreamerlin changed the title [Refactor] Polish eval hook [Refactor] refactor EvalHook Dec 19, 2020
@dreamerlin dreamerlin marked this pull request as ready for review December 22, 2020 09:02
@dreamerlin dreamerlin requested a review from innerlee December 22, 2020 09:03
irvingzhang0512
irvingzhang0512 reviewed Dec 24, 2020
View reviewed changes
@dreamerlin dreamerlin mentioned this pull request Dec 24, 2020
Merged
@irvingzhang0512 irvingzhang0512 mentioned this pull request Dec 26, 2020
Closed
@irvingzhang0512 irvingzhang0512 mentioned this pull request Jan 14, 2021
Merged
congee524
congee524 reviewed Jan 25, 2021
View reviewed changes
congee524
congee524 reviewed Jan 25, 2021
View reviewed changes
@dreamerlin dreamerlin force-pushed the polish_eval_hook branch 2 times, most recently from 8b0b623 to 5267285 Compare January 25, 2021 17:20
@innerlee
Copy link
Copy Markdown
Contributor

innerlee commented Jan 27, 2021

Please help with user migration.

  • Add a stub for the old classes DistEpochEvalHook, EpochEvalHook, and give proper error messages (ideally, also give links to this pr) to guide the user to update configs/codes.
  • Give detailed migration guide on the top post in this thread. Like I commented at Refactor ava hook #567 (comment)

@dreamerlin
Copy link
Copy Markdown
Collaborator Author

dreamerlin commented Jan 27, 2021

Please help with user migration.

  • Add a stub for the old classes DistEpochEvalHook, EpochEvalHook, and give proper error messages (ideally, also give links to this pr) to guide the user to update configs/codes.
  • Give detailed migration guide on the top post in this thread. Like I commented at #567 (comment)

Done

@innerlee innerlee merged commit 4240105 into open-mmlab:master Jan 27, 2021
@dreamerlin dreamerlin deleted the polish_eval_hook branch January 29, 2021 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers

@irvingzhang0512 irvingzhang0512 irvingzhang0512 left review comments

@congee524 congee524 congee524 left review comments

@innerlee innerlee innerlee approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

breaking

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dreamerlin @innerlee @irvingzhang0512 @congee524