Support optimizers with different parameters

[DavdGao]

• This PR is to solve the issue A problem when using Adam optimizer #91
• Solution
  • Specific the parameters of the local optimizer by adding new parameters under the config cfg.optimizer and cfg.fedopt.optimizer. :
  • The calling of get_optimizer is as follows

    optimizer = get_optimizer(model=model, **cfg.optimizer)

• Example:
  • Taking cfg.optimizer as an example, the original config file is as follows

    # ------------------------------------------------------------------------ #
    # Optimizer related options
    # ------------------------------------------------------------------------ #
    cfg.optimizer = CN(new_allowed=True)

    cfg.optimizer.type = 'SGD'
    cfg.optimizer.lr = 0.1

• By setting new_allowed=True in cfg.optimizer, we allow the users to add new parameters according to the type of their optimizers. For example, if I want to use the optimizer registered as myoptimizer, as well as its new parameters mylr and mynorm. I just need to write the yaml file as follows, and the new parameters will be added automatically.

optimizer:
    type: myoptimizer
    mylr: 0.1
    mynorm: 1

[joneswong]

I am wondering whether it is necessary to change the exposed parameters. To resolve this issue, why not just change the argument of get_optimizer? For example, let the caller pass cfg.optimizer instead of each of the optional ones (e.g., weight_decay and momentum), and, in get_optimizer(), pack them into the appropriate kwargs regarding type of optimizer.

[joneswong]

I am wondering whether it is necessary to change the exposed parameters. To resolve this issue, why not just change the argument of get_optimizer? For example, let the caller pass cfg.optimizer instead of each of the optional ones (e.g., weight_decay and momentum), and, in get_optimizer(), pack them into the appropriate kwargs regarding type of optimizer.

what is your opinion? @DavdGao @rayrayraykk @yxdyc @xieyxclack

[rayrayraykk]

I am wondering whether it is necessary to change the exposed parameters. To resolve this issue, why not just change the argument of get_optimizer? For example, let the caller pass cfg.optimizer instead of each of the optional ones (e.g., weight_decay and momentum), and, in get_optimizer(), pack them into the appropriate kwargs regarding type of optimizer.

what is your opinion? @DavdGao @rayrayraykk @yxdyc @xieyxclack

Agreed, we could use a filter to pass the args:

FederatedScope/federatedscope/core/auxiliaries/data_builder.py

Line 155 in e625a0d

def filter_dict(func, kwarg):

This function can be applied to optimizers too.

[DavdGao]

I am wondering whether it is necessary to change the exposed parameters. To resolve this issue, why not just change the argument of get_optimizer? For example, let the caller pass cfg.optimizer instead of each of the optional ones (e.g., weight_decay and momentum), and, in get_optimizer(), pack them into the appropriate kwargs regarding type of optimizer.

what is your opinion? @DavdGao @rayrayraykk @yxdyc @xieyxclack

The solution is updated accordingly.

[DavdGao]

I am wondering whether it is necessary to change the exposed parameters. To resolve this issue, why not just change the argument of get_optimizer? For example, let the caller pass cfg.optimizer instead of each of the optional ones (e.g., weight_decay and momentum), and, in get_optimizer(), pack them into the appropriate kwargs regarding type of optimizer.

what is your opinion? @DavdGao @rayrayraykk @yxdyc @xieyxclack

Agreed, we could use a filter to pass the args:

FederatedScope/federatedscope/core/auxiliaries/data_builder.py

Line 155 in e625a0d

def filter_dict(func, kwarg):

This function can be applied to optimizers too.

The solution is updated accordingly.

[joneswong]

This configuring mechanism looks cool to me! Could you provide a test case for it?

[DavdGao]

This configuring mechanism looks cool to me! Could you provide a test case for it?

Thanks, the unittest is added.

[yxdyc]

LGTM, plz see the inline comments

[yxdyc]

Should we ensure forward compatibility? That is, both optimizer.grad_clip or grad.grad_clip are ok. Otherwise, we may have to exhaustively modify the historical codes to ensure that the previous experiments still work correctly

[DavdGao]

• @yxdyc Thanks, I have checked the historical codes and make sure that all unittest works well.
• As for gradient clipping, since it is not a common parameter in general optimizers (like the learning rate), maybe we should consider it as an independent operation and separate it from the optimizer.
• Since we have set cfg.optimizer = CN(new_allowed=True), our modification also supports optimizer.grad_clip as a parameter for customized optimziers.

[yxdyc]

The same problem as "optimizer.grad_clip vs. grad.grad_clip" above.

[joneswong]

LGTM