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

[Refactor] refactor EvalHook #395

Merged
merged 16 commits into from
Jan 27, 2021
Merged

Conversation

dreamerlin
Copy link
Collaborator

@dreamerlin dreamerlin commented Nov 28, 2020

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='[email protected]')

to

evaluation = dict(interval=1, save_best='[email protected]')

Misc.

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

@codecov
Copy link

codecov bot commented Nov 28, 2020

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 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
@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
Contributor

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
Collaborator Author

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
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants