Support help and required argument for the configs #294

yxdyc · 2022-08-08T05:08:35Z

As the title said, now we can set argument as

cfg.fedopt.optimizer.type = Argument(
        'SGD', description="optimizer type for FedOPT")

And the main.py will print help info as follows:

xieyxclack

LGTM, please see the inline comments for minor suggestions. THX!

xieyxclack · 2022-08-08T05:49:53Z

federatedscope/attack/auxiliary/attack_trainer_builder.py

@@ -3,7 +3,7 @@ def wrap_attacker_trainer(base_trainer, config):
    Args:
        base_trainer (core.trainers.GeneralTorchTrainer): the trainer that
        will be wrapped;
-        config (yacs.config.CfgNode): the configure;
+        config (core.configs.config.CN): the configure;


core.configs -> federatedscope.core.configs?

xieyxclack · 2022-08-08T05:50:29Z

federatedscope/autotune/algos.py

@@ -68,7 +67,7 @@ def run(self):
 def get_scheduler(init_cfg):
    """To instantiate an scheduler object for conducting HPO
    Arguments:
-        init_cfg (yacs.Node): configuration.
+        init_cfg (core.configs.config.CN): configuration.


Same as above

xieyxclack · 2022-08-08T05:50:36Z

federatedscope/autotune/algos.py

@@ -88,8 +87,8 @@ class Scheduler(object):
    def __init__(self, cfg):
        """
            Arguments:
-                cfg (yacs.Node): dict like object, where each key-value pair
-                corresponds to a field and its choices.
+                cfg (ycore.configs.config.CN): dict like object, where each


xieyxclack · 2022-08-08T05:51:53Z

federatedscope/core/cmd_args.py

    parser.add_argument('opts',
                        help='See federatedscope/core/configs for all options',
                        default=None,
                        nargs=argparse.REMAINDER)
-    if len(sys.argv) == 1:
+    parse_res = parser.parse_args(args)
+    from federatedscope.core.configs.config import global_cfg


Maybe we should move the import to the top of the file

xieyxclack · 2022-08-08T05:53:54Z

federatedscope/core/cmd_args.py

+    parse_res = parser.parse_args(args)
+    from federatedscope.core.configs.config import global_cfg
+    init_cfg = global_cfg.clone()
+    if len(sys.argv) == 1 or parse_res.help == "all":


We can add some comments for these if sentences

xieyxclack · 2022-08-08T05:56:59Z

materials/notebook/01_quick_start.ipynb

@@ -23,7 +23,10 @@
   },
   "source": [
    "## Configurations\n",
-    "**FederatedScope** organize the configuration through `yacs.config.cfgNode` and can be found in `federatedscope.configs`. Please refer to the [official documentation](https://federatedscope.io/refs/index) for specific instructions on how to configure `cfg`."
+    "**FederatedScope** organize the configuration through `core.configs.config


a formatting issue?

xieyxclack · 2022-08-08T05:57:12Z

materials/notebook/03_personalized_FL.ipynb

@@ -49,7 +49,9 @@
    "For more larger datasets, please refer examples in `scripts/personalization_exp_scripts`.\n",
    "\n",
    "### Configuration\n",
-    "**FederatedScope** organizes the configuration through an extension of `yacs.config.cfgNode`. Please refer to our [official documentation](https://federatedscope.io/docs/own-case/) for specific instructions on how to configure the initial `global_cfg` and customize your own configuration. By default, we provide built-in personalization-related configurations in `federatedscope/core/configs/`."
+    "**FederatedScope** organizes the configuration through an extension of


Same as above

xieyxclack · 2022-08-08T05:58:05Z

federatedscope/core/configs/yacs_config.py

@@ -0,0 +1,600 @@
+# An extended configuration system based on [yacs],


We need to add these to LICENSE too

LGTM, please see the inline comments for minor suggestions. THX!

I made modifications according to the comments

- modified according to yuexiang's comments.

xieyxclack · 2022-08-08T10:24:33Z

LICENSE

+Code in federatedscope/core/configs/yacs_config.py, the basic code of yacs
+adopts Apache 2.0 License
+
+# Copyright (c) 2018-present, Facebook, Inc.


- modified according to yuexiang's comments

DavdGao · 2022-08-09T02:48:25Z

@yxdyc Here is some of my concerns:

Does it mean that we need to set the arguments by python federatedscope/main.py --fedopt.optimizer.lr==0.1 in the future?
Is it compatible with the old scripts? e.g., python federatedscope/main.py fedopt.optimizer=0.1
In current master branch some namespaces support customized arguments. For example, users can add new arguments under the namespace train.optimizer, finetune.optimizer and fedopt.optimizer (we have set new_allowed=True), which haven't been pre-defined in our config files. I'm not sure if the new mechanism still supports it.

yxdyc · 2022-08-10T08:37:45Z

@yxdyc Here is some of my concerns:

Does it mean that we need to set the arguments by python federatedscope/main.py --fedopt.optimizer.lr==0.1 in the future?

Is it compatible with the old scripts? e.g., python federatedscope/main.py fedopt.optimizer=0.1

In current master branch some namespaces support customized arguments. For example, users can add new arguments under the namespace train.optimizer, finetune.optimizer and fedopt.optimizer (we have set new_allowed=True), which haven't been pre-defined in our config files. I'm not sure if the new mechanism still supports it.

Thanks for the questions. I have added de_arguments for compatibility with historical codes

xieyxclack

LGTM, we can add descriptions of configs later. Thanks!

- support help and required argument for the configs - added de-arguments for compatibility with historical codes

support help and required argument for the configs

ff8298f

yxdyc added the enhancement New feature or request label Aug 8, 2022

yxdyc requested review from xieyxclack and DavdGao August 8, 2022 05:08

xieyxclack reviewed Aug 8, 2022

View reviewed changes

- bugfix for unittest

6995f4d

- modified according to yuexiang's comments.

xieyxclack reviewed Aug 8, 2022

View reviewed changes

- bugfix for unittest

a345643

- modified according to yuexiang's comments

added de-arguments for compatibility with historical codes

e0430ea

bug fix for fedopt unittest

6e64140

xieyxclack approved these changes Aug 11, 2022

View reviewed changes

xieyxclack merged commit dd012a8 into alibaba:master Aug 11, 2022

Schichael pushed a commit to Schichael/FederatedScope_thesis that referenced this pull request Sep 7, 2022

Support help and required argument for the configs (alibaba#294)

8b64d89

- support help and required argument for the configs - added de-arguments for compatibility with historical codes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support help and required argument for the configs #294

Support help and required argument for the configs #294

yxdyc commented Aug 8, 2022

xieyxclack left a comment

xieyxclack Aug 8, 2022

xieyxclack Aug 8, 2022

xieyxclack Aug 8, 2022

xieyxclack Aug 8, 2022

xieyxclack Aug 8, 2022

xieyxclack Aug 8, 2022

xieyxclack Aug 8, 2022

xieyxclack Aug 8, 2022

yxdyc Aug 8, 2022

xieyxclack Aug 8, 2022

DavdGao commented Aug 9, 2022

yxdyc commented Aug 10, 2022

xieyxclack left a comment

		@@ -0,0 +1,600 @@
		# An extended configuration system based on [yacs],

Support help and required argument for the configs #294

Support help and required argument for the configs #294

Conversation

yxdyc commented Aug 8, 2022

xieyxclack left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DavdGao commented Aug 9, 2022

yxdyc commented Aug 10, 2022

xieyxclack left a comment

Choose a reason for hiding this comment