Custom Hook Issue and Fix Proposal

[lorinczszabolcs]

Checklist

1. I have searched related issues but cannot get the expected help. ✅
2. The unexpected results still exist in the latest version. ✅

Describe the Issue Tried to add a custom hook with

cfg.custom_hooks = [dict(type="DenseCLHook")]

but got

File "/opt/conda/lib/python3.8/site-packages/mmselfsup/apis/train.py", line 174, in train_model
    if hook_cfg.type == 'DeepClusterHook':
AttributeError: 'dict' object has no attribute 'type'

Bug fix I think the line https://github.com/open-mmlab/mmselfsup/blob/abcd43b4318e6fcef8111b10dce3146bc78019eb/mmselfsup/apis/train.py#L173 should be altered to if hook_cfg.get('type', None) == 'DeepClusterHook'

[fangyixiao18]

Hello, where did you add the code cfg.custom_hooks = [dict(type="DenseCLHook")]

[lorinczszabolcs]

Hi! Thanks for your quick reply!

Just before calling mmselfsup.apis.train_model:

train_model(
    model,
    datasets,
    cfg,
    distributed=False,
    timestamp=time.strftime("%Y%m%d_%H%M%S", time.localtime()),
    meta=dict(),
)

Nevertheless, here https://github.com/open-mmlab/mmselfsup/blob/abcd43b4318e6fcef8111b10dce3146bc78019eb/mmselfsup/apis/train.py#L169-L173

it shows that the hook_cfg has to be a dict, but dicts do not support dot notation for accessing values in a dictionary as far as I know, so I think my suggestion to change if hook_cfg.type == 'DeepClusterHook': to if hook_cfg.get('type', None) == 'DeepClusterHook' to would be valid no matter the use case in my opinion.

[fangyixiao18]

Actually, as for this configuration, it is recommended to modify the config file

The example:

_base_ = [
    '../_base_/models/densecl.py',
    '../_base_/datasets/imagenet_mocov2.py',
    '../_base_/schedules/sgd_coslr-200e_in1k.py',
    '../_base_/default_runtime.py',
]

# runtime settings
custom_hooks = [dict(type='DenseCLHook')]
# the max_keep_ckpts controls the max number of ckpt file in your work_dirs
# if it is 3, when CheckpointHook (in mmcv) saves the 4th ckpt
# it will remove the oldest one to keep the number of total ckpts as 3
checkpoint_config = dict(interval=10, max_keep_ckpts=3)

[lorinczszabolcs]

If I want to set that hook based on a flag that is provided to my main script, that won't work unfortunately.

Also, it won't change the fact that the custom hook is a dictionary, so its 'type' can not be accessed by calling hook_cfg.type. There are two options: either hook_cfg['type'] or hook_cfg.get('type', None)

This way no custom hook can be added, because once the custom hooks list contains any dictionary, it will run in this issue.

[fangyixiao18]

If I want to set that hook based on a flag that is provided to my main script, that won't work unfortunately.

Also, it won't change the fact that the custom hook is a dictionary, so its 'type' can not be accessed by calling hook_cfg.type. There are two options: either hook_cfg['type'] or hook_cfg.get('type', None)

This way no custom hook can be added, because once the custom hooks list contains any dictionary, it will run in this issue.

Welcome to create a PR to help us revise this part ！ Thanks in advance.

[lorinczszabolcs]

Created a PR, let me know if I can do anything else. :)