From 8608c45b641e474e435126bba32b972512e04967 Mon Sep 17 00:00:00 2001
From: "gaodawei.gdw" <gaodawei.gdw@alibaba-inc.com>
Date: Wed, 25 May 2022 10:11:28 +0800
Subject: [PATCH 1/6] minor change

---
 .../core/auxiliaries/optimizer_builder.py     |  2 +-
 federatedscope/core/configs/cfg_training.py   |  7 ++--
 federatedscope/core/configs/config.py         | 14 +++----
 federatedscope/core/trainers/context.py       |  6 +--
 .../run_femnist_dp_standalone.sh              | 37 -------------------
 .../run_femnist_fedopt_standalone.sh          | 19 ----------
 .../run_femnist_standard_standalone.sh        | 17 ---------
 7 files changed, 14 insertions(+), 88 deletions(-)
 delete mode 100644 scripts/dp_exp_scrips/run_femnist_dp_standalone.sh
 delete mode 100644 scripts/dp_exp_scrips/run_femnist_fedopt_standalone.sh
 delete mode 100644 scripts/dp_exp_scrips/run_femnist_standard_standalone.sh

diff --git a/federatedscope/core/auxiliaries/optimizer_builder.py b/federatedscope/core/auxiliaries/optimizer_builder.py
index a001787cf..12f5e9fcd 100644
--- a/federatedscope/core/auxiliaries/optimizer_builder.py
+++ b/federatedscope/core/auxiliaries/optimizer_builder.py
@@ -4,7 +4,7 @@
     torch = None
 
 
-def get_optimizer(type, model, lr, **kwargs):
+def get_optimizer(model, type, lr, grad_clip, **kwargs):
     if torch is None:
         return None
     if isinstance(type, str):
diff --git a/federatedscope/core/configs/cfg_training.py b/federatedscope/core/configs/cfg_training.py
index 3a9537cdc..b2b487023 100644
--- a/federatedscope/core/configs/cfg_training.py
+++ b/federatedscope/core/configs/cfg_training.py
@@ -19,12 +19,13 @@ def extend_training_cfg(cfg):
     # ------------------------------------------------------------------------ #
     # Optimizer related options
     # ------------------------------------------------------------------------ #
-    cfg.optimizer = CN()
+    cfg.optimizer = CN(new_allowed=True)
 
     cfg.optimizer.type = 'SGD'
     cfg.optimizer.lr = 0.1
-    cfg.optimizer.weight_decay = .0
-    cfg.optimizer.momentum = .0
+
+
+    # cfg.grad.clip = -1.0
     cfg.optimizer.grad_clip = -1.0  # negative numbers indicate we do not clip grad
 
     # ------------------------------------------------------------------------ #
diff --git a/federatedscope/core/configs/config.py b/federatedscope/core/configs/config.py
index f92ac31ef..03edeef70 100644
--- a/federatedscope/core/configs/config.py
+++ b/federatedscope/core/configs/config.py
@@ -14,7 +14,13 @@ class CN(CfgNode):
     """
     def __init__(self, init_dict=None, key_list=None, new_allowed=False):
         super().__init__(init_dict, key_list, new_allowed)
-        self.cfg_check_funcs = []  # to check the config values validity
+        self.__dict__["cfg_check_funcs"] = list() # to check the config values validity
+
+    def __getattr__(self, name):
+        if name in self:
+            return self[name]
+        else:
+            raise AttributeError(name)
 
     def register_cfg_check_fun(self, cfg_check_fun):
         self.cfg_check_funcs.append(cfg_check_fun)
@@ -26,9 +32,7 @@ def merge_from_file(self, cfg_filename):
         :param cfg_filename (string):
         :return:
         """
-        cfg_check_funcs = copy.copy(self.cfg_check_funcs)
         super(CN, self).merge_from_file(cfg_filename)
-        self.cfg_check_funcs = cfg_check_funcs
         self.assert_cfg()
 
     def merge_from_other_cfg(self, cfg_other):
@@ -38,9 +42,7 @@ def merge_from_other_cfg(self, cfg_other):
         :param cfg_other (CN):
         :return:
         """
-        cfg_check_funcs = copy.copy(self.cfg_check_funcs)
         super(CN, self).merge_from_other_cfg(cfg_other)
-        self.cfg_check_funcs = cfg_check_funcs
         self.assert_cfg()
 
     def merge_from_list(self, cfg_list):
@@ -50,9 +52,7 @@ def merge_from_list(self, cfg_list):
         :param cfg_list (list):
         :return:
         """
-        cfg_check_funcs = copy.copy(self.cfg_check_funcs)
         super(CN, self).merge_from_list(cfg_list)
-        self.cfg_check_funcs = cfg_check_funcs
         self.assert_cfg()
 
     def assert_cfg(self):
diff --git a/federatedscope/core/trainers/context.py b/federatedscope/core/trainers/context.py
index 5783ccfe7..8185fec4d 100644
--- a/federatedscope/core/trainers/context.py
+++ b/federatedscope/core/trainers/context.py
@@ -101,11 +101,9 @@ def setup_vars(self):
                                            self.device)
             self.regularizer = get_regularizer(self.cfg.regularizer.type)
             self.optimizer = get_optimizer(
-                self.cfg.optimizer.type,
                 self.model,
-                self.cfg.optimizer.lr,
-                weight_decay=self.cfg.optimizer.weight_decay,
-                momentum=self.cfg.optimizer.momentum)
+                **self.cfg.optimizer
+            )
             self.grad_clip = self.cfg.optimizer.grad_clip
         elif self.cfg.backend == 'tensorflow':
             self.trainable_para_names = self.model.trainable_variables()
diff --git a/scripts/dp_exp_scrips/run_femnist_dp_standalone.sh b/scripts/dp_exp_scrips/run_femnist_dp_standalone.sh
deleted file mode 100644
index 347fce5df..000000000
--- a/scripts/dp_exp_scrips/run_femnist_dp_standalone.sh
+++ /dev/null
@@ -1,37 +0,0 @@
-set -e
-
-cudaid=$1
-
-if [ ! -d "out_dp" ];then
-  mkdir out_dp
-fi
-
-echo "DP starts..."
-
-clips=(0.1)
-epsilons=(100.)
-mus=(0.1)
-constants=(2. 3. 4.)
-
-for ((iw=0; iw<${#clips[@]}; iw++ ))
-do
-    for ((ie=0; ie<${#epsilons[@]}; ie++ ))
-    do
-        for ((im=0; im<${#mus[@]}; im++ ))
-        do
-            for ((ic=0; ic<${#constants[@]}; ic++ ))
-            do
-                python federatedscope/main.py --cfg federatedscope/cv/baseline/fedavg_convnet2_on_femnist.yaml device ${cudaid} nbafl.use True \
-                nbafl.mu ${mus[$im]} \
-                nbafl.epsilon ${epsilons[$ie]} \
-                nbafl.constant ${constants[$ic]} \
-                nbafl.w_clip ${clips[$iw]} \
-                >>out_dp/clip_${clips[$iw]}_eps_${epsilons[$ie]}_mu_${mus[$im]}_const_${constants[$ic]}.out \
-                2>>out_dp/clip_${clips[$iw]}_eps_${epsilons[$ie]}_mu_${mus[$im]}_const_${constants[$ic]}.err
-            done
-        done
-    done
-done
-
-echo "Ends."
-
diff --git a/scripts/dp_exp_scrips/run_femnist_fedopt_standalone.sh b/scripts/dp_exp_scrips/run_femnist_fedopt_standalone.sh
deleted file mode 100644
index 21623c18f..000000000
--- a/scripts/dp_exp_scrips/run_femnist_fedopt_standalone.sh
+++ /dev/null
@@ -1,19 +0,0 @@
-set -e
-
-cudaid=$1
-
-if [ ! -d "out_fedopt" ];then
-  mkdir out_fedopt
-fi
-
-echo "Starts..."
-
-python federatedscope/main.py --cfg federatedscope/cv/baseline/fedavg_convnet2_on_femnist.yaml \
-device ${cudaid} federate.method fedopt\
->>out_fedopt/fedopt.out \
-2>>out_fedopt/fedopt.err
-
-python federatedscope/parse_exp_results.py --input out_fedopt/fedopt.out
-
-echo "Ends."
-
diff --git a/scripts/dp_exp_scrips/run_femnist_standard_standalone.sh b/scripts/dp_exp_scrips/run_femnist_standard_standalone.sh
deleted file mode 100644
index 193feda98..000000000
--- a/scripts/dp_exp_scrips/run_femnist_standard_standalone.sh
+++ /dev/null
@@ -1,17 +0,0 @@
-set -e
-
-cudaid=$1
-
-if [ ! -d "out_dp" ];then
-  mkdir out_dp
-fi
-
-echo "Starts..."
-
-python federatedscope/main.py --cfg federatedscope/cv/baseline/fedavg_convnet2_on_femnist.yaml \
-device ${cudaid} \
->>out_dp/standard.out \
-2>>out_dp/standard.err
-
-echo "Ends."
-

From cc31622f62fe1e8399429c28e46a9ce672f6fe6f Mon Sep 17 00:00:00 2001
From: "gaodawei.gdw" <gaodawei.gdw@alibaba-inc.com>
Date: Wed, 25 May 2022 11:18:58 +0800
Subject: [PATCH 2/6] permit to add new parameters in `cfg.optimizer` and
 `cfg.fedopt.optimizer`

---
 .../attack/worker_as_attacker/server_attacker.py          | 6 +++---
 federatedscope/core/aggregator.py                         | 6 ++----
 federatedscope/core/auxiliaries/optimizer_builder.py      | 2 +-
 federatedscope/core/configs/cfg_fl_algo.py                | 8 +++++---
 federatedscope/core/configs/cfg_training.py               | 8 +++++---
 federatedscope/core/trainers/context.py                   | 2 +-
 federatedscope/core/trainers/trainer_Ditto.py             | 8 ++------
 federatedscope/core/trainers/trainer_multi_model.py       | 6 ++----
 .../cv/baseline/fedavg_convnet2_on_femnist.yaml           | 1 +
 federatedscope/cv/baseline/fedbn_convnet2_on_femnist.yaml | 1 +
 federatedscope/example_configs/femnist.yaml               | 1 +
 scripts/fedopt_exp_scripts/run_fedopt_femnist.sh          | 2 +-
 scripts/fedopt_exp_scripts/run_fedopt_lr.sh               | 2 +-
 scripts/fedopt_exp_scripts/run_fedopt_shakespeare.sh      | 2 +-
 scripts/gnn_exp_scripts/run_graph_level_opt.sh            | 2 +-
 scripts/gnn_exp_scripts/run_link_level_opt.sh             | 2 +-
 scripts/gnn_exp_scripts/run_multi_opt.sh                  | 2 +-
 scripts/gnn_exp_scripts/run_node_level_opt.sh             | 2 +-
 .../ditto/ditto_convnet2_on_femnist.yaml                  | 1 +
 .../fedbn/fedbn_convnet2_on_femnist.yaml                  | 1 +
 .../fedem/fedem_convnet2_on_femnist.yaml                  | 1 +
 .../pfedme/pfedme_convnet2_on_femnist.yaml                | 1 +
 tests/test_ditto.py                                       | 2 +-
 tests/test_external_dataset.py                            | 2 +-
 tests/test_fedem.py                                       | 2 +-
 tests/test_fedopt.py                                      | 2 +-
 tests/test_femnist.py                                     | 2 +-
 tests/test_nbafl.py                                       | 2 +-
 tests/test_pfedme.py                                      | 2 +-
 29 files changed, 42 insertions(+), 39 deletions(-)

diff --git a/federatedscope/attack/worker_as_attacker/server_attacker.py b/federatedscope/attack/worker_as_attacker/server_attacker.py
index 97c1f7e24..f94a074b3 100644
--- a/federatedscope/attack/worker_as_attacker/server_attacker.py
+++ b/federatedscope/attack/worker_as_attacker/server_attacker.py
@@ -164,14 +164,14 @@ def __init__(self,
             fl_model_criterion=get_criterion(self._cfg.criterion.type,
                                              device=self.device),
             device=self.device,
-            grad_clip=self._cfg.optimizer.grad_clip,
+            grad_clip=self._cfg.grad.grad_clip,
             dataset_name=self._cfg.data.type,
             fl_local_update_num=self._cfg.federate.local_update_steps,
-            fl_type_optimizer=self._cfg.fedopt.type_optimizer,
+            fl_type_optimizer=self._cfg.fedopt.optimizer.type,
             fl_lr=self._cfg.optimizer.lr,
             batch_size=100)
 
-        # self.optimizer = get_optimizer(type=self._cfg.fedopt.type_optimizer, model=self.model,lr=self._cfg.fedopt.lr_server)
+        # self.optimizer = get_optimizer(type=self._cfg.fedopt.type_optimizer, model=self.model,lr=self._cfg.fedopt.optimizer.lr)
         # print(self.optimizer)
     def callback_funcs_model_para(self, message: Message):
         round, sender, content = message.state, message.sender, message.content
diff --git a/federatedscope/core/aggregator.py b/federatedscope/core/aggregator.py
index 428b7b257..9da4b556c 100644
--- a/federatedscope/core/aggregator.py
+++ b/federatedscope/core/aggregator.py
@@ -174,10 +174,8 @@ def __init__(self, config, model, device='cpu'):
         self.cfg = config
         self.model = model
         self.device = device
-        self.optimizer = get_optimizer(type=config.fedopt.type_optimizer,
-                                       model=self.model,
-                                       lr=config.fedopt.lr_server,
-                                       momentum=config.fedopt.momentum_server)
+        self.optimizer = get_optimizer(model=self.model,
+                                       **config.fedopt.optimizer)
 
     def aggregate(self, agg_info):
         new_model = super().aggregate(agg_info)
diff --git a/federatedscope/core/auxiliaries/optimizer_builder.py b/federatedscope/core/auxiliaries/optimizer_builder.py
index 12f5e9fcd..6083dbaee 100644
--- a/federatedscope/core/auxiliaries/optimizer_builder.py
+++ b/federatedscope/core/auxiliaries/optimizer_builder.py
@@ -4,7 +4,7 @@
     torch = None
 
 
-def get_optimizer(model, type, lr, grad_clip, **kwargs):
+def get_optimizer(model, type, lr, **kwargs):
     if torch is None:
         return None
     if isinstance(type, str):
diff --git a/federatedscope/core/configs/cfg_fl_algo.py b/federatedscope/core/configs/cfg_fl_algo.py
index ac312a104..c727b6d94 100644
--- a/federatedscope/core/configs/cfg_fl_algo.py
+++ b/federatedscope/core/configs/cfg_fl_algo.py
@@ -9,9 +9,11 @@ def extend_fl_algo_cfg(cfg):
     cfg.fedopt = CN()
 
     cfg.fedopt.use = False
-    cfg.fedopt.lr_server = 0.01
-    cfg.fedopt.momentum_server = 0.
-    cfg.fedopt.type_optimizer = 'SGD'
+
+    cfg.fedopt.optimizer = CN(new_allowed=True)
+    cfg.fedopt.optimizer.type = 'SGD'
+    cfg.fedopt.optimizer.lr = 0.01
+    cfg.fedopt.optimizer.momentum = 0.
 
     # ------------------------------------------------------------------------ #
     # fedprox related options, general fl
diff --git a/federatedscope/core/configs/cfg_training.py b/federatedscope/core/configs/cfg_training.py
index b2b487023..eaa75dcd4 100644
--- a/federatedscope/core/configs/cfg_training.py
+++ b/federatedscope/core/configs/cfg_training.py
@@ -24,9 +24,11 @@ def extend_training_cfg(cfg):
     cfg.optimizer.type = 'SGD'
     cfg.optimizer.lr = 0.1
 
-
-    # cfg.grad.clip = -1.0
-    cfg.optimizer.grad_clip = -1.0  # negative numbers indicate we do not clip grad
+    # ------------------------------------------------------------------------ #
+    # Gradient related options
+    # ------------------------------------------------------------------------ #
+    cfg.grad = CN()
+    cfg.grad.grad_clip = -1.0  # negative numbers indicate we do not clip grad
 
     # ------------------------------------------------------------------------ #
     # lr_scheduler related options
diff --git a/federatedscope/core/trainers/context.py b/federatedscope/core/trainers/context.py
index 8185fec4d..e20526943 100644
--- a/federatedscope/core/trainers/context.py
+++ b/federatedscope/core/trainers/context.py
@@ -104,7 +104,7 @@ def setup_vars(self):
                 self.model,
                 **self.cfg.optimizer
             )
-            self.grad_clip = self.cfg.optimizer.grad_clip
+            self.grad_clip = self.cfg.grad.grad_clip
         elif self.cfg.backend == 'tensorflow':
             self.trainable_para_names = self.model.trainable_variables()
             self.criterion = None
diff --git a/federatedscope/core/trainers/trainer_Ditto.py b/federatedscope/core/trainers/trainer_Ditto.py
index 0699f7072..312500648 100644
--- a/federatedscope/core/trainers/trainer_Ditto.py
+++ b/federatedscope/core/trainers/trainer_Ditto.py
@@ -63,15 +63,11 @@ def init_Ditto_ctx(base_trainer):
     ctx.local_model = copy.deepcopy(ctx.model)  # the personalized model
 
     ctx.optimizer_for_global_model = get_optimizer(
-        cfg.optimizer.type,
         ctx.global_model,
-        cfg.optimizer.lr,
-        weight_decay=cfg.optimizer.weight_decay)
+        **cfg.optimizer)
     ctx.optimizer_for_local_model = get_optimizer(
-        cfg.optimizer.type,
         ctx.local_model,
-        cfg.personalization.lr,
-        weight_decay=cfg.optimizer.weight_decay)
+        **cfg.optimizer)
     ctx.optimizer_for_local_model = wrap_regularized_optimizer(
         ctx.optimizer_for_local_model, cfg.personalization.regular_weight)
 
diff --git a/federatedscope/core/trainers/trainer_multi_model.py b/federatedscope/core/trainers/trainer_multi_model.py
index ea0c4946d..ffc5627fe 100644
--- a/federatedscope/core/trainers/trainer_multi_model.py
+++ b/federatedscope/core/trainers/trainer_multi_model.py
@@ -96,10 +96,8 @@ def init_multiple_models(self):
         self.ctx.models = [self.ctx.model] + additional_models
 
         additional_optimizers = [
-            get_optimizer(self.cfg.optimizer.type,
-                          self.ctx.models[i],
-                          self.cfg.optimizer.lr,
-                          weight_decay=self.cfg.optimizer.weight_decay)
+            get_optimizer(self.ctx.models[i],
+                          **self.cfg.optimizer)
             for i in range(1, self.model_nums)
         ]
         self.ctx.optimizers = [self.ctx.optimizer] + additional_optimizers
diff --git a/federatedscope/cv/baseline/fedavg_convnet2_on_femnist.yaml b/federatedscope/cv/baseline/fedavg_convnet2_on_femnist.yaml
index d0e7beedb..16e686730 100644
--- a/federatedscope/cv/baseline/fedavg_convnet2_on_femnist.yaml
+++ b/federatedscope/cv/baseline/fedavg_convnet2_on_femnist.yaml
@@ -25,6 +25,7 @@ model:
 optimizer:
   lr: 0.01
   weight_decay: 0.0
+grad:
   grad_clip: 5.0
 criterion:
   type: CrossEntropyLoss
diff --git a/federatedscope/cv/baseline/fedbn_convnet2_on_femnist.yaml b/federatedscope/cv/baseline/fedbn_convnet2_on_femnist.yaml
index 9a6e8263a..bd956e34d 100644
--- a/federatedscope/cv/baseline/fedbn_convnet2_on_femnist.yaml
+++ b/federatedscope/cv/baseline/fedbn_convnet2_on_femnist.yaml
@@ -27,6 +27,7 @@ personalization:
 optimizer:
   lr: 0.01
   weight_decay: 0.0
+grad:
   grad_clip: 5.0
 criterion:
   type: CrossEntropyLoss
diff --git a/federatedscope/example_configs/femnist.yaml b/federatedscope/example_configs/femnist.yaml
index cb7b657cf..bde9a589a 100644
--- a/federatedscope/example_configs/femnist.yaml
+++ b/federatedscope/example_configs/femnist.yaml
@@ -24,6 +24,7 @@ model:
 optimizer:
   lr: 0.01
   weight_decay: 0.0
+grad:
   grad_clip: 5.0
 criterion:
   type: CrossEntropyLoss
diff --git a/scripts/fedopt_exp_scripts/run_fedopt_femnist.sh b/scripts/fedopt_exp_scripts/run_fedopt_femnist.sh
index e0dbddbbd..f8b748ac8 100644
--- a/scripts/fedopt_exp_scripts/run_fedopt_femnist.sh
+++ b/scripts/fedopt_exp_scripts/run_fedopt_femnist.sh
@@ -16,7 +16,7 @@ do
   data.root /mnt/gaodawei.gdw/data/ \
   fedopt.use True \
   federate.method FedOpt \
-  fedopt.lr_server ${lrs[$il]} \
+  fedopt.optimizer.lr ${lrs[$il]} \
   >>out_fedopt_femnist/nothing.out \
   2>>out_fedopt_femnist/lr_${lrs[$il]}.log
 done
diff --git a/scripts/fedopt_exp_scripts/run_fedopt_lr.sh b/scripts/fedopt_exp_scripts/run_fedopt_lr.sh
index dea47ada9..72a895ce9 100644
--- a/scripts/fedopt_exp_scripts/run_fedopt_lr.sh
+++ b/scripts/fedopt_exp_scripts/run_fedopt_lr.sh
@@ -16,7 +16,7 @@ do
   data.root /mnt/gaodawei.gdw/data/ \
   fedopt.use True \
   federate.method FedOpt \
-  fedopt.lr_server ${lrs[$il]} \
+  fedopt.optimizer.lr ${lrs[$il]} \
   >>out_fedopt_lr/nothing.out \
   2>>out_fedopt_lr/lr_${lrs[$il]}.log
 done
diff --git a/scripts/fedopt_exp_scripts/run_fedopt_shakespeare.sh b/scripts/fedopt_exp_scripts/run_fedopt_shakespeare.sh
index 5bda5fe2c..4c51721f1 100644
--- a/scripts/fedopt_exp_scripts/run_fedopt_shakespeare.sh
+++ b/scripts/fedopt_exp_scripts/run_fedopt_shakespeare.sh
@@ -16,7 +16,7 @@ do
   data.root /mnt/gaodawei.gdw/data/ \
   fedopt.use True \
   federate.method FedOpt \
-  fedopt.lr_server ${lrs[$il]} \
+  fedopt.optimizer.lr ${lrs[$il]} \
   >>out_fedopt_shakespeare/nothing.out \
   2>>out_fedopt_shakespeare/lr_${lrs[$il]}.log
 done
diff --git a/scripts/gnn_exp_scripts/run_graph_level_opt.sh b/scripts/gnn_exp_scripts/run_graph_level_opt.sh
index e0f19e968..e959a238b 100644
--- a/scripts/gnn_exp_scripts/run_graph_level_opt.sh
+++ b/scripts/gnn_exp_scripts/run_graph_level_opt.sh
@@ -36,7 +36,7 @@ for (( s=0; s<${#lr_servers[@]}; s++ ))
 do
     for k in {1..5}
     do
-        python federatedscope/main.py --cfg federatedscope/gfl/baseline/fedavg_gcn_minibatch_on_hiv.yaml device ${cudaid} data.type ${dataset} data.splitter ${splitter} optimizer.lr ${lr} federate.local_update_steps ${local_update} model.type ${gnn} model.out_channels ${out_channels} model.hidden ${hidden} seed $k federate.method FedOpt fedopt.lr_server ${lr_servers[$s]} >>out/${gnn}_${lr}_${local_update}_on_${dataset}_${splitter}_${lr_servers[$s]}_opt.log 2>&1
+        python federatedscope/main.py --cfg federatedscope/gfl/baseline/fedavg_gcn_minibatch_on_hiv.yaml device ${cudaid} data.type ${dataset} data.splitter ${splitter} optimizer.lr ${lr} federate.local_update_steps ${local_update} model.type ${gnn} model.out_channels ${out_channels} model.hidden ${hidden} seed $k federate.method FedOpt fedopt.optimizer.lr ${lr_servers[$s]} >>out/${gnn}_${lr}_${local_update}_on_${dataset}_${splitter}_${lr_servers[$s]}_opt.log 2>&1
     done
 done
 
diff --git a/scripts/gnn_exp_scripts/run_link_level_opt.sh b/scripts/gnn_exp_scripts/run_link_level_opt.sh
index bb76c7ffd..38b3e169b 100644
--- a/scripts/gnn_exp_scripts/run_link_level_opt.sh
+++ b/scripts/gnn_exp_scripts/run_link_level_opt.sh
@@ -36,7 +36,7 @@ for (( s=0; s<${#lr_servers[@]}; s++ ))
 do
     for k in {1..5}
     do
-        python federatedscope/main.py --cfg federatedscope/gfl/baseline/fedavg_gcn_fullbatch_on_kg.yaml device ${cudaid} data.type ${dataset} data.splitter ${splitter} optimizer.lr ${lr} federate.local_update_steps ${local_update} model.type ${gnn} model.out_channels ${out_channels} model.hidden ${hidden} seed $k federate.method FedOpt fedopt.lr_server ${lr_servers[$s]} model.layer ${layer}  >>out/${gnn}_${lr}_${local_update}_on_${dataset}_${splitter}_${lr_servers[$s]}_opt.log 2>&1
+        python federatedscope/main.py --cfg federatedscope/gfl/baseline/fedavg_gcn_fullbatch_on_kg.yaml device ${cudaid} data.type ${dataset} data.splitter ${splitter} optimizer.lr ${lr} federate.local_update_steps ${local_update} model.type ${gnn} model.out_channels ${out_channels} model.hidden ${hidden} seed $k federate.method FedOpt fedopt.optimizer.lr ${lr_servers[$s]} model.layer ${layer}  >>out/${gnn}_${lr}_${local_update}_on_${dataset}_${splitter}_${lr_servers[$s]}_opt.log 2>&1
     done
 done
 
diff --git a/scripts/gnn_exp_scripts/run_multi_opt.sh b/scripts/gnn_exp_scripts/run_multi_opt.sh
index 546a5680d..92aa64cce 100644
--- a/scripts/gnn_exp_scripts/run_multi_opt.sh
+++ b/scripts/gnn_exp_scripts/run_multi_opt.sh
@@ -34,7 +34,7 @@ for (( s=0; s<${#lr_servers[@]}; s++ ))
 do
     for k in {1..5}
     do
-        python federatedscope/main.py --cfg federatedscope/gfl/baseline/fedavg_gnn_minibatch_on_multi_task.yaml device ${cudaid} data.type ${dataset} data.splitter ${splitter} optimizer.lr ${lr} federate.local_update_steps ${local_update} model.type ${gnn} model.out_channels ${out_channels} model.hidden ${hidden} seed $k federate.method FedOpt fedopt.lr_server ${lr_servers[$s]} >>out/${gnn}_${lr}_${local_update}_on_${dataset}_${splitter}_${lr_servers[$s]}_opt.log 2>&1
+        python federatedscope/main.py --cfg federatedscope/gfl/baseline/fedavg_gnn_minibatch_on_multi_task.yaml device ${cudaid} data.type ${dataset} data.splitter ${splitter} optimizer.lr ${lr} federate.local_update_steps ${local_update} model.type ${gnn} model.out_channels ${out_channels} model.hidden ${hidden} seed $k federate.method FedOpt fedopt.optimizer.lr ${lr_servers[$s]} >>out/${gnn}_${lr}_${local_update}_on_${dataset}_${splitter}_${lr_servers[$s]}_opt.log 2>&1
     done
 done
 
diff --git a/scripts/gnn_exp_scripts/run_node_level_opt.sh b/scripts/gnn_exp_scripts/run_node_level_opt.sh
index 071b18d08..c79663043 100644
--- a/scripts/gnn_exp_scripts/run_node_level_opt.sh
+++ b/scripts/gnn_exp_scripts/run_node_level_opt.sh
@@ -39,7 +39,7 @@ for (( s=0; s<${#lr_servers[@]}; s++ ))
 do
     for k in {1..5}
     do
-        python federatedscope/main.py --cfg federatedscope/gfl/baseline/fedavg_gnn_node_fullbatch_citation.yaml device ${cudaid} data.type ${dataset} data.splitter ${splitter} optimizer.lr ${lr} federate.local_update_steps ${local_update} model.type ${gnn} model.out_channels ${out_channels} model.hidden ${hidden} seed $k federate.method FedOpt fedopt.lr_server ${lr_servers[$s]} model.layer ${layer} >>out/${gnn}_${lr}_${local_update}_on_${dataset}_${splitter}_${lr_servers[$s]}_opt.log 2>&1
+        python federatedscope/main.py --cfg federatedscope/gfl/baseline/fedavg_gnn_node_fullbatch_citation.yaml device ${cudaid} data.type ${dataset} data.splitter ${splitter} optimizer.lr ${lr} federate.local_update_steps ${local_update} model.type ${gnn} model.out_channels ${out_channels} model.hidden ${hidden} seed $k federate.method FedOpt fedopt.optimizer.lr ${lr_servers[$s]} model.layer ${layer} >>out/${gnn}_${lr}_${local_update}_on_${dataset}_${splitter}_${lr_servers[$s]}_opt.log 2>&1
     done
 done
 
diff --git a/scripts/personalization_exp_scripts/ditto/ditto_convnet2_on_femnist.yaml b/scripts/personalization_exp_scripts/ditto/ditto_convnet2_on_femnist.yaml
index 512dbacbd..bacab7107 100644
--- a/scripts/personalization_exp_scripts/ditto/ditto_convnet2_on_femnist.yaml
+++ b/scripts/personalization_exp_scripts/ditto/ditto_convnet2_on_femnist.yaml
@@ -28,6 +28,7 @@ personalization:
 optimizer:
   lr: 0.5
   weight_decay: 0.0
+grad:
   grad_clip: 5.0
 criterion:
   type: CrossEntropyLoss
diff --git a/scripts/personalization_exp_scripts/fedbn/fedbn_convnet2_on_femnist.yaml b/scripts/personalization_exp_scripts/fedbn/fedbn_convnet2_on_femnist.yaml
index c65d43cb0..bc454736d 100644
--- a/scripts/personalization_exp_scripts/fedbn/fedbn_convnet2_on_femnist.yaml
+++ b/scripts/personalization_exp_scripts/fedbn/fedbn_convnet2_on_femnist.yaml
@@ -26,6 +26,7 @@ personalization:
 optimizer:
   lr: 0.1
   weight_decay: 0.0
+grad:
   grad_clip: 5.0
 criterion:
   type: CrossEntropyLoss
diff --git a/scripts/personalization_exp_scripts/fedem/fedem_convnet2_on_femnist.yaml b/scripts/personalization_exp_scripts/fedem/fedem_convnet2_on_femnist.yaml
index 8c452d2a1..7b62b5440 100644
--- a/scripts/personalization_exp_scripts/fedem/fedem_convnet2_on_femnist.yaml
+++ b/scripts/personalization_exp_scripts/fedem/fedem_convnet2_on_femnist.yaml
@@ -28,6 +28,7 @@ personalization:
 optimizer:
   lr: 0.5
   weight_decay: 0.0
+grad:
   grad_clip: 5.0
 criterion:
   type: CrossEntropyLoss
diff --git a/scripts/personalization_exp_scripts/pfedme/pfedme_convnet2_on_femnist.yaml b/scripts/personalization_exp_scripts/pfedme/pfedme_convnet2_on_femnist.yaml
index ab23d8473..511d23551 100644
--- a/scripts/personalization_exp_scripts/pfedme/pfedme_convnet2_on_femnist.yaml
+++ b/scripts/personalization_exp_scripts/pfedme/pfedme_convnet2_on_femnist.yaml
@@ -28,6 +28,7 @@ personalization:
 optimizer:
   lr: 0.5
   weight_decay: 0.0
+grad:
   grad_clip: 5.0
 criterion:
   type: CrossEntropyLoss
diff --git a/tests/test_ditto.py b/tests/test_ditto.py
index 516cb65b7..25f6a5a27 100644
--- a/tests/test_ditto.py
+++ b/tests/test_ditto.py
@@ -48,7 +48,7 @@ def set_config_femnist(self, cfg):
 
         cfg.optimizer.lr = 0.001
         cfg.optimizer.weight_decay = 0.0
-        cfg.optimizer.grad_clip = 5.0
+        cfg.grad.grad_clip = 5.0
 
         cfg.criterion.type = 'CrossEntropyLoss'
         cfg.trainer.type = 'cvtrainer'
diff --git a/tests/test_external_dataset.py b/tests/test_external_dataset.py
index f4db55519..a923595d8 100644
--- a/tests/test_external_dataset.py
+++ b/tests/test_external_dataset.py
@@ -48,7 +48,7 @@ def set_config_torchvision_dataset(self, cfg):
 
         cfg.optimizer.lr = 0.01
         cfg.optimizer.weight_decay = 0.0
-        cfg.optimizer.grad_clip = 5.0
+        cfg.grad.grad_clip = 5.0
 
         cfg.criterion.type = 'CrossEntropyLoss'
         cfg.trainer.type = 'cvtrainer'
diff --git a/tests/test_fedem.py b/tests/test_fedem.py
index a6fe473ed..be0c47da5 100644
--- a/tests/test_fedem.py
+++ b/tests/test_fedem.py
@@ -48,7 +48,7 @@ def set_config_femnist(self, cfg):
 
         cfg.optimizer.lr = 0.001
         cfg.optimizer.weight_decay = 0.0
-        cfg.optimizer.grad_clip = 5.0
+        cfg.grad.grad_clip = 5.0
 
         cfg.criterion.type = 'CrossEntropyLoss'
         cfg.trainer.type = 'cvtrainer'
diff --git a/tests/test_fedopt.py b/tests/test_fedopt.py
index 1ee2c0176..c783868bd 100644
--- a/tests/test_fedopt.py
+++ b/tests/test_fedopt.py
@@ -51,7 +51,7 @@ def set_config_fedopt(self, cfg):
         cfg.trainer.type = 'cvtrainer'
         cfg.seed = 123
 
-        cfg.fedopt.lr_server = 1.
+        cfg.fedopt.optimizer.lr = 1.
 
         return backup_cfg
 
diff --git a/tests/test_femnist.py b/tests/test_femnist.py
index 273ef2c2e..e9e8c033d 100644
--- a/tests/test_femnist.py
+++ b/tests/test_femnist.py
@@ -45,7 +45,7 @@ def set_config_femnist(self, cfg):
 
         cfg.optimizer.lr = 0.001
         cfg.optimizer.weight_decay = 0.0
-        cfg.optimizer.grad_clip = 5.0
+        cfg.grad.grad_clip = 5.0
 
         cfg.criterion.type = 'CrossEntropyLoss'
         cfg.trainer.type = 'cvtrainer'
diff --git a/tests/test_nbafl.py b/tests/test_nbafl.py
index e39c1094b..618c40040 100644
--- a/tests/test_nbafl.py
+++ b/tests/test_nbafl.py
@@ -44,7 +44,7 @@ def set_config_femnist(self, cfg):
 
         cfg.optimizer.lr = 0.001
         cfg.optimizer.weight_decay = 0.0
-        cfg.optimizer.grad_clip = 5.0
+        cfg.grad.grad_clip = 5.0
 
         cfg.criterion.type = 'CrossEntropyLoss'
         cfg.trainer.type = 'cvtrainer'
diff --git a/tests/test_pfedme.py b/tests/test_pfedme.py
index e68744bc9..65ad9e168 100644
--- a/tests/test_pfedme.py
+++ b/tests/test_pfedme.py
@@ -50,7 +50,7 @@ def set_config_femnist(self, cfg):
 
         cfg.optimizer.lr = 0.001
         cfg.optimizer.weight_decay = 0.0
-        cfg.optimizer.grad_clip = 5.0
+        cfg.grad.grad_clip = 5.0
 
         cfg.criterion.type = 'CrossEntropyLoss'
         cfg.trainer.type = 'cvtrainer'

From df9ab25cf1ab313f15cae4a4f87aaecc9eb40739 Mon Sep 17 00:00:00 2001
From: "gaodawei.gdw" <gaodawei.gdw@alibaba-inc.com>
Date: Wed, 25 May 2022 11:22:40 +0800
Subject: [PATCH 3/6] bug fix

---
 federatedscope/core/configs/cfg_fl_algo.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/federatedscope/core/configs/cfg_fl_algo.py b/federatedscope/core/configs/cfg_fl_algo.py
index c727b6d94..1ff1996e3 100644
--- a/federatedscope/core/configs/cfg_fl_algo.py
+++ b/federatedscope/core/configs/cfg_fl_algo.py
@@ -13,7 +13,6 @@ def extend_fl_algo_cfg(cfg):
     cfg.fedopt.optimizer = CN(new_allowed=True)
     cfg.fedopt.optimizer.type = 'SGD'
     cfg.fedopt.optimizer.lr = 0.01
-    cfg.fedopt.optimizer.momentum = 0.
 
     # ------------------------------------------------------------------------ #
     # fedprox related options, general fl

From 7631c2500dd98790f360fc52d23afc5766e324f0 Mon Sep 17 00:00:00 2001
From: "gaodawei.gdw" <gaodawei.gdw@alibaba-inc.com>
Date: Wed, 25 May 2022 11:35:44 +0800
Subject: [PATCH 4/6] run format.sh

---
 federatedscope/core/configs/config.py               |  3 ++-
 federatedscope/core/trainers/context.py             |  5 +----
 federatedscope/core/trainers/trainer_Ditto.py       | 10 ++++------
 federatedscope/core/trainers/trainer_multi_model.py |  3 +--
 4 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/federatedscope/core/configs/config.py b/federatedscope/core/configs/config.py
index 03edeef70..ce8755302 100644
--- a/federatedscope/core/configs/config.py
+++ b/federatedscope/core/configs/config.py
@@ -14,7 +14,8 @@ class CN(CfgNode):
     """
     def __init__(self, init_dict=None, key_list=None, new_allowed=False):
         super().__init__(init_dict, key_list, new_allowed)
-        self.__dict__["cfg_check_funcs"] = list() # to check the config values validity
+        self.__dict__["cfg_check_funcs"] = list(
+        )  # to check the config values validity
 
     def __getattr__(self, name):
         if name in self:
diff --git a/federatedscope/core/trainers/context.py b/federatedscope/core/trainers/context.py
index e20526943..b6c63a986 100644
--- a/federatedscope/core/trainers/context.py
+++ b/federatedscope/core/trainers/context.py
@@ -100,10 +100,7 @@ def setup_vars(self):
             self.criterion = get_criterion(self.cfg.criterion.type,
                                            self.device)
             self.regularizer = get_regularizer(self.cfg.regularizer.type)
-            self.optimizer = get_optimizer(
-                self.model,
-                **self.cfg.optimizer
-            )
+            self.optimizer = get_optimizer(self.model, **self.cfg.optimizer)
             self.grad_clip = self.cfg.grad.grad_clip
         elif self.cfg.backend == 'tensorflow':
             self.trainable_para_names = self.model.trainable_variables()
diff --git a/federatedscope/core/trainers/trainer_Ditto.py b/federatedscope/core/trainers/trainer_Ditto.py
index 312500648..34d502aea 100644
--- a/federatedscope/core/trainers/trainer_Ditto.py
+++ b/federatedscope/core/trainers/trainer_Ditto.py
@@ -62,12 +62,10 @@ def init_Ditto_ctx(base_trainer):
     ctx.global_model = copy.deepcopy(ctx.model)
     ctx.local_model = copy.deepcopy(ctx.model)  # the personalized model
 
-    ctx.optimizer_for_global_model = get_optimizer(
-        ctx.global_model,
-        **cfg.optimizer)
-    ctx.optimizer_for_local_model = get_optimizer(
-        ctx.local_model,
-        **cfg.optimizer)
+    ctx.optimizer_for_global_model = get_optimizer(ctx.global_model,
+                                                   **cfg.optimizer)
+    ctx.optimizer_for_local_model = get_optimizer(ctx.local_model,
+                                                  **cfg.optimizer)
     ctx.optimizer_for_local_model = wrap_regularized_optimizer(
         ctx.optimizer_for_local_model, cfg.personalization.regular_weight)
 
diff --git a/federatedscope/core/trainers/trainer_multi_model.py b/federatedscope/core/trainers/trainer_multi_model.py
index ffc5627fe..1448de991 100644
--- a/federatedscope/core/trainers/trainer_multi_model.py
+++ b/federatedscope/core/trainers/trainer_multi_model.py
@@ -96,8 +96,7 @@ def init_multiple_models(self):
         self.ctx.models = [self.ctx.model] + additional_models
 
         additional_optimizers = [
-            get_optimizer(self.ctx.models[i],
-                          **self.cfg.optimizer)
+            get_optimizer(self.ctx.models[i], **self.cfg.optimizer)
             for i in range(1, self.model_nums)
         ]
         self.ctx.optimizers = [self.ctx.optimizer] + additional_optimizers

From 6de2cc3278a86c7d3acce55ea77ee8dc536397a8 Mon Sep 17 00:00:00 2001
From: "gaodawei.gdw" <gaodawei.gdw@alibaba-inc.com>
Date: Wed, 25 May 2022 13:03:29 +0800
Subject: [PATCH 5/6] bug fix

---
 federatedscope/core/configs/config.py | 32 +++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/federatedscope/core/configs/config.py b/federatedscope/core/configs/config.py
index ce8755302..d3bf7ee48 100644
--- a/federatedscope/core/configs/config.py
+++ b/federatedscope/core/configs/config.py
@@ -2,6 +2,8 @@
 import os
 
 from yacs.config import CfgNode
+from yacs.config import _assert_with_logging
+from yacs.config import _check_and_coerce_cfg_value_type
 
 import federatedscope.register as register
 
@@ -49,11 +51,37 @@ def merge_from_other_cfg(self, cfg_other):
     def merge_from_list(self, cfg_list):
         """
            load configs from a list stores the keys and values.
+           modified `merge_from_list` in `yacs.config.py` to allow adding new keys if `is_new_allowed()` returns True
 
         :param cfg_list (list):
         :return:
         """
-        super(CN, self).merge_from_list(cfg_list)
+        _assert_with_logging(
+            len(cfg_list) % 2 == 0,
+            "Override list has odd length: {}; it must be a list of pairs".format(
+                cfg_list
+            ),
+        )
+        root = self
+        for full_key, v in zip(cfg_list[0::2], cfg_list[1::2]):
+            if root.key_is_deprecated(full_key):
+                continue
+            if root.key_is_renamed(full_key):
+                root.raise_key_rename_error(full_key)
+            key_list = full_key.split(".")
+            d = self
+            for subkey in key_list[:-1]:
+                _assert_with_logging(
+                    subkey in d, "Non-existent key: {}".format(full_key)
+                )
+                d = d[subkey]
+            subkey = key_list[-1]
+            _assert_with_logging(subkey in d or d.is_new_allowed(), "Non-existent key: {}".format(full_key))
+            value = self._decode_cfg_value(v)
+            if subkey in d:
+                value = _check_and_coerce_cfg_value_type(value, d[subkey], subkey, full_key)
+            d[subkey] = value
+
         self.assert_cfg()
 
     def assert_cfg(self):
@@ -95,7 +123,7 @@ def freeze(self):
             from contextlib import redirect_stdout
             with redirect_stdout(outfile):
                 tmp_cfg = copy.deepcopy(self)
-                tmp_cfg.cfg_check_funcs = []
+                tmp_cfg.cfg_check_funcs.clear()
                 print(tmp_cfg.dump())
 
         super(CN, self).freeze()

From c6bbaa02f30177bec61acc79b13727b3395ab8d6 Mon Sep 17 00:00:00 2001
From: "gaodawei.gdw" <gaodawei.gdw@alibaba-inc.com>
Date: Tue, 31 May 2022 10:20:46 +0800
Subject: [PATCH 6/6] add a unit test for the config of optimizer

---
 tests/test_optimizer.py | 82 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)
 create mode 100644 tests/test_optimizer.py

diff --git a/tests/test_optimizer.py b/tests/test_optimizer.py
new file mode 100644
index 000000000..1af832011
--- /dev/null
+++ b/tests/test_optimizer.py
@@ -0,0 +1,82 @@
+# Copyright (c) Alibaba, Inc. and its affiliates.
+import unittest
+
+from federatedscope.core.auxiliaries.data_builder import get_data
+from federatedscope.core.auxiliaries.utils import setup_seed, update_logger
+from federatedscope.core.configs.config import global_cfg
+from federatedscope.core.fed_runner import FedRunner
+from federatedscope.core.auxiliaries.worker_builder import get_server_cls, get_client_cls
+
+
+class FEMNISTTest(unittest.TestCase):
+    def setUp(self):
+        print(('Testing %s.%s' % (type(self).__name__, self._testMethodName)))
+
+    def set_config_femnist(self, cfg):
+        backup_cfg = cfg.clone()
+
+        import torch
+        cfg.use_gpu = torch.cuda.is_available()
+        cfg.eval.freq = 10
+        cfg.eval.metrics = ['acc', 'loss_regular']
+
+        cfg.federate.mode = 'standalone'
+        cfg.federate.local_update_steps = 5
+        cfg.federate.total_round_num = 20
+        cfg.federate.sample_client_num = 5
+        cfg.federate.client_num = 10
+
+        cfg.data.root = 'test_data/'
+        cfg.data.type = 'femnist'
+        cfg.data.splits = [0.6, 0.2, 0.2]
+        cfg.data.batch_size = 10
+        cfg.data.subsample = 0.05
+        cfg.data.transform = [['ToTensor'],
+                              [
+                                  'Normalize', {
+                                      'mean': [0.1307],
+                                      'std': [0.3081]
+                                  }
+                              ]]
+
+        cfg.model.type = 'convnet2'
+        cfg.model.hidden = 2048
+        cfg.model.out_channels = 62
+
+        cfg.optimizer.type = "Adam"
+        cfg.optimizer.lr = 0.001
+        cfg.optimizer.betas = [0.9, 0.999]
+        cfg.optimizer.weight_decay = 0.001
+        cfg.grad.grad_clip = 5.0
+
+        cfg.criterion.type = 'CrossEntropyLoss'
+        cfg.trainer.type = 'cvtrainer'
+        cfg.seed = 123
+
+        return backup_cfg
+
+    def test_femnist_standalone(self):
+        init_cfg = global_cfg.clone()
+        backup_cfg = self.set_config_femnist(init_cfg)
+        setup_seed(init_cfg.seed)
+        update_logger(init_cfg)
+
+        data, modified_cfg = get_data(init_cfg.clone())
+        init_cfg.merge_from_other_cfg(modified_cfg)
+        self.assertIsNotNone(data)
+
+        Fed_runner = FedRunner(data=data,
+                               server_class=get_server_cls(init_cfg),
+                               client_class=get_client_cls(init_cfg),
+                               config=init_cfg.clone())
+        self.assertIsNotNone(Fed_runner)
+        test_best_results = Fed_runner.run()
+        print(test_best_results)
+        init_cfg.merge_from_other_cfg(backup_cfg)
+        self.assertLess(
+            test_best_results["client_summarized_weighted_avg"]['test_loss'],
+            600)
+
+
+if __name__ == '__main__':
+    unittest.main()