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

Errors occur in LrUpdaterHook when multiple optimizers are introduced #887

Closed
wuyuuu opened this issue Mar 15, 2021 · 7 comments
Closed
Assignees

Comments

@wuyuuu
Copy link

wuyuuu commented Mar 15, 2021

Hi,

I'm using mmpose with multiple optimizers and I got errors in LrUpdateHook

Traceback (most recent call last):
  File "/home2/wuy/.pycharm_helpers/pydev/pydevd.py", line 1477, in _exec
    pydev_imports.execfile(file, globals, locals)  # execute the script
  File "/home2/wuy/.pycharm_helpers/pydev/_pydev_imps/_pydev_execfile.py", line 18, in execfile
    exec(compile(contents+"\n", file, 'exec'), glob, loc)
  File "/home2/wuy/project/mmpose/tools/model_search.py", line 169, in <module>
    main()
  File "/home2/wuy/project/mmpose/tools/model_search.py", line 165, in main
    meta=meta)
  File "/home2/wuy/project/mmpose/mmpose/apis/model_search.py", line 151, in search_model
    runner.run(data_loaders, cfg.workflow, cfg.total_epochs)
  File "/home2/wuy/project/mmpose/mmpose/apis/my_epoch_runner.py", line 134, in run
    epoch_runner(data_loaders[i], **kwargs)
  File "/home2/wuy/project/mmpose/mmpose/apis/my_epoch_runner.py", line 51, in train
    self.call_hook('before_train_iter')
  File "/home2/wuy/anaconda3/lib/python3.7/site-packages/mmcv/runner/base_runner.py", line 308, in call_hook
    getattr(hook, fn_name)(self)
  File "/home2/wuy/anaconda3/lib/python3.7/site-packages/mmcv/runner/hooks/lr_updater.py", line 139, in before_train_iter
    warmup_lr = self.get_warmup_lr(cur_iter)
  File "/home2/wuy/anaconda3/lib/python3.7/site-packages/mmcv/runner/hooks/lr_updater.py", line 88, in get_warmup_lr
    warmup_lr = [_lr * (1 - k) for _lr in self.regular_lr]
  File "/home2/wuy/anaconda3/lib/python3.7/site-packages/mmcv/runner/hooks/lr_updater.py", line 88, in <listcomp>
    warmup_lr = [_lr * (1 - k) for _lr in self.regular_lr]
TypeError: can't multiply sequence by non-int of type 'float'

It seems to me it is because get_warmup_lr does not work for multiple optimizers as self.regular_lr is a list for single optimizer while it is a dict for multiple optimizers.

def get_warmup_lr(self, cur_iters):
if self.warmup == 'constant':
warmup_lr = [_lr * self.warmup_ratio for _lr in self.regular_lr]
elif self.warmup == 'linear':
k = (1 - cur_iters / self.warmup_iters) * (1 - self.warmup_ratio)
warmup_lr = [_lr * (1 - k) for _lr in self.regular_lr]
elif self.warmup == 'exp':
k = self.warmup_ratio**(1 - cur_iters / self.warmup_iters)
warmup_lr = [_lr * k for _lr in self.regular_lr]
return warmup_lr

def get_regular_lr(self, runner):
if isinstance(runner.optimizer, dict):
lr_groups = {}
for k in runner.optimizer.keys():
_lr_group = [
self.get_lr(runner, _base_lr)
for _base_lr in self.base_lr[k]
]
lr_groups.update({k: _lr_group})
return lr_groups
else:
return [self.get_lr(runner, _base_lr) for _base_lr in self.base_lr]

@zhouzaida
Copy link
Collaborator

zhouzaida commented Mar 15, 2021

please show the optimizer which you define.
I guess _lr may be str.

@wuyuuu
Copy link
Author

wuyuuu commented Mar 15, 2021

please show the optimizer which you define.
I guess _lr may be str.

Thanks for your prompt reply, your guess is correct.
my optimizer is as the form: dict('model1': torch.optim.Optimizer, 'model2': torch.optim.Optimizer)

{'model': SGD (
Parameter Group 0
    dampening: 0
    lr: 0.001
    momentum: 0
    nesterov: False
    weight_decay: 0
), 'model2': SGD (
Parameter Group 0
    dampening: 0
    lr: 0.001
    momentum: 0
    nesterov: False
    weight_decay: 0
)}

BTW, self.regular_lr is {'model': [0.001], 'model2': [0.001]}
and _lr becomes 'model1' and 'model2'

@zhouzaida
Copy link
Collaborator

Should MomentumUpdaterHook also support multiple optimizers like the LRUpdaterHook?

def get_regular_momentum(self, runner):
return [
self.get_momentum(runner, _base_momentum)
for _base_momentum in self.base_momentum
]

def get_regular_lr(self, runner):
if isinstance(runner.optimizer, dict):
lr_groups = {}
for k in runner.optimizer.keys():
_lr_group = [
self.get_lr(runner, _base_lr)
for _base_lr in self.base_lr[k]
]
lr_groups.update({k: _lr_group})
return lr_groups
else:
return [self.get_lr(runner, _base_lr) for _base_lr in self.base_lr]

@wuyuuu
Copy link
Author

wuyuuu commented Mar 21, 2021

Should MomentumUpdaterHook also support multiple optimizers like the LRUpdaterHook?

def get_regular_momentum(self, runner):
return [
self.get_momentum(runner, _base_momentum)
for _base_momentum in self.base_momentum
]

def get_regular_lr(self, runner):
if isinstance(runner.optimizer, dict):
lr_groups = {}
for k in runner.optimizer.keys():
_lr_group = [
self.get_lr(runner, _base_lr)
for _base_lr in self.base_lr[k]
]
lr_groups.update({k: _lr_group})
return lr_groups
else:
return [self.get_lr(runner, _base_lr) for _base_lr in self.base_lr]

Hello Zhou,

To my best knowledge, I think the MomentumHook should set different momentums for different optimizers. ( Now I'm using Adam optimizer for my project which incorporates the momentum and I think surely different Adam optimizers should include different momentums)

BTW, I simply modified the LrUpdateHook by:

        def get_warmup_lr(self, cur_iters):
        if isinstance(self.regular_lr, dict):
            w = [_lr[0] for _lr in self.regular_lr.values()]
        else:
            w = self.regular_lr
        if self.warmup == 'constant':
            warmup_lr = [_lr * self.warmup_ratio for _lr in w]
        elif self.warmup == 'linear':
            k = (1 - cur_iters / self.warmup_iters) * (1 - self.warmup_ratio)
            warmup_lr = [_lr * (1 - k) for _lr in w]
        elif self.warmup == 'exp':
            k = self.warmup_ratio ** (1 - cur_iters / self.warmup_iters)
            warmup_lr = [_lr * k for _lr in w]
        if isinstance(self.regular_lr, dict):
            return {k: [_lr] for k, _lr in zip(self.regular_lr.keys(), warmup_lr)}
        return warmup_lr

So far it works well for me.

@zhouzaida
Copy link
Collaborator

Should MomentumUpdaterHook also support multiple optimizers like the LRUpdaterHook?

def get_regular_momentum(self, runner):
return [
self.get_momentum(runner, _base_momentum)
for _base_momentum in self.base_momentum
]

def get_regular_lr(self, runner):
if isinstance(runner.optimizer, dict):
lr_groups = {}
for k in runner.optimizer.keys():
_lr_group = [
self.get_lr(runner, _base_lr)
for _base_lr in self.base_lr[k]
]
lr_groups.update({k: _lr_group})
return lr_groups
else:
return [self.get_lr(runner, _base_lr) for _base_lr in self.base_lr]

Hello Zhou,

To my best knowledge, I think the MomentumHook should set different momentums for different optimizers. ( Now I'm using Adam optimizer for my project which incorporates the momentum and I think surely different Adam optimizers should include different momentums)

BTW, I simply modified the LrUpdateHook by:

        def get_warmup_lr(self, cur_iters):
        if isinstance(self.regular_lr, dict):
            w = [_lr[0] for _lr in self.regular_lr.values()]
        else:
            w = self.regular_lr
        if self.warmup == 'constant':
            warmup_lr = [_lr * self.warmup_ratio for _lr in w]
        elif self.warmup == 'linear':
            k = (1 - cur_iters / self.warmup_iters) * (1 - self.warmup_ratio)
            warmup_lr = [_lr * (1 - k) for _lr in w]
        elif self.warmup == 'exp':
            k = self.warmup_ratio ** (1 - cur_iters / self.warmup_iters)
            warmup_lr = [_lr * k for _lr in w]
        if isinstance(self.regular_lr, dict):
            return {k: [_lr] for k, _lr in zip(self.regular_lr.keys(), warmup_lr)}
        return warmup_lr

So far it works well for me.

Thanks for your feedback, we will support what you suggest in the next version.

@zhouzaida
Copy link
Collaborator

Related PR #907

@zhouzaida
Copy link
Collaborator

hi, @wuyuuu, the issue is resolved by pr #907 merged into master.

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

No branches or pull requests

3 participants