EvalHook uses case-insensitive key indicator matching and configurabl…

[ly015]

Motivation

Make the EvalHook more flexible to ease the inheritance and usage of it in the downstream repos.

Modification

1. Use case-insensitive key indicator matching to infer the comparing rule.
2. Use a configurable test function that can be passed as an initialization parameter. If not set, the default test function "mmcv.engine.single_gpu_test" (or "mmcv.engine.multi_gpu_test" for DistEvalHook) will be used.

Usage

Config

To use EvalHook with a specific setting, users need to modify the evaluation variable in config files. like:

evaluation = dict(interval=10, metric='mAP', save_best='AP')

Migration

Since the key for determine greater an less is related to the downstream task, downstream repos may need to overwrite the self.greater_keys and self.less_keys:
It is recommended to implement EvalHook in downstream repos by inheriting the one in MMCV. Specifically, downstream repos may need to:

1. overwrite self.greater_keys and self.less_keys. Note that these keys will be used in a case-insensitive way, so it's better to define them in lower case for consistency.
2. override self.__init__ to specify the test_fn. Or the default ones (mmcv.engine.single_gpu_test for EvalHook and mmcv.engine.multi_gpu_test for DistEvalHook)

For example:

# mmpose/core/evaluation/eval_hooks.py

from mmcv.runner import EvalHook as _EvalHook

class EvalHook(_EvalHook):
    greater_keys = ['acc', 'ap', 'pck']
    less_keys = ['loss', 'epe', 'mpjpe']

    def __init__(self, data_loader, start=None, interval=1, by_epoch=True, save_best=None, rule=None, **eval_kwargs):
        from mmpose.apis import single_gpu_test
        super().__init__(data_loader, start, interval, by_epoch, save_best, test_fn=single_gpu_test, **eval_kwargs)

BC-breaking (Optional)

May need to check if there are any case-sensitive key indicators used in downstream repos.

[CLAassistant]

All committers have signed the CLA.

[codecov]

Codecov Report

Merging #1076 (a99f30d) into master (d212bd5) will increase coverage by 1.94%.
The diff coverage is 77.41%.

@@            Coverage Diff             @@
##           master    #1076      +/-   ##
==========================================
+ Coverage   65.72%   67.66%   +1.94%     
==========================================
  Files         157      159       +2     
  Lines       10092    10296     +204     
  Branches     1828     1864      +36     
==========================================
+ Hits         6633     6967     +334     
+ Misses       3113     2961     -152     
- Partials      346      368      +22     

Flag	Coverage Δ
unittests	67.66% <77.41%> (+1.94%)	⬆️

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

Impacted Files	Coverage Δ
mmcv/runner/hooks/evaluation.py	86.70% <77.41%> (-1.46%)	⬇️
mmcv/ops/multi_scale_deform_attn.py	58.40% <0.00%> (-11.29%)	⬇️
mmcv/utils/ext_loader.py	35.89% <0.00%> (-2.34%)	⬇️
mmcv/runner/fp16_utils.py	58.12% <0.00%> (-1.61%)	⬇️
mmcv/runner/hooks/optimizer.py	16.66% <0.00%> (-1.08%)	⬇️
mmcv/ops/__init__.py	100.00% <0.00%> (ø)
mmcv/cnn/bricks/__init__.py	100.00% <0.00%> (ø)
mmcv/cnn/bricks/registry.py	100.00% <0.00%> (ø)
mmcv/runner/hooks/logger/__init__.py	100.00% <0.00%> (ø)
mmcv/cnn/bricks/drop.py	100.00% <0.00%> (ø)
... and 7 more

---

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 d212bd5...a99f30d. Read the comment docs.

[dreamerlin]

Usage and Migration are needed in the PR description for downstream repos, like #739

[ly015]

@dreamerlin
Usage and Migration parts are added to the PR message. Please have a look.

[dreamerlin]

LGTM, What about other repos @ZwwWayne, @LXXXXR, @innerlee

[ZwwWayne]

How about we do this once during initialization rather than import the function every epoch?

[ly015]

Kindly ping. @xvjiarui @ZwwWayne