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

Improve Syntax of Sigils #6721

Closed
wyli opened this issue Jul 14, 2023 · 6 comments · Fixed by #6955
Closed

Improve Syntax of Sigils #6721

wyli opened this issue Jul 14, 2023 · 6 comments · Fixed by #6955
Assignees

Comments

@wyli
Copy link
Contributor

wyli commented Jul 14, 2023

Using the # character as a sigil or separator is slightly cumbersome because it's also the comment character in Python and Bash. Values mentioned on the command line need to be put in quotes, eg. '--trainer#max_epochs' 5. Perhaps deprecating this and using : or :: instead?

ID_REF_KEY = "@" # start of a reference to a ConfigItem
ID_SEP_KEY = "#" # separator for the ID of a ConfigItem
EXPR_KEY = "$" # start of a ConfigExpression
MACRO_KEY = "%" # start of a macro of a config

@wyli
Copy link
Contributor Author

wyli commented Aug 31, 2023

what would be the preferred character? : and :: are used in slicing so they might be confusing as well. cc @ericspod @Nic-Ma @KumoLiu

the current # may not be highlighted properly in a bash script:
Screenshot 2023-08-31 at 13 27 06

@KumoLiu
Copy link
Contributor

KumoLiu commented Sep 1, 2023

How about ^^? I didn't find it being used here.

@ericspod
Copy link
Member

ericspod commented Sep 1, 2023

Although ":" is used in slicing I think it would be clear here that the use is different. "^" and "^^" are used for changing case in bash so that might conflict. I would also avoid "|" since this is pipe in bash. Perhaps "!" wouldn't conflict with anything?

It might well be possible to use "." as a separator, it would mean either member access in place of "#" or what "." means normally and it would be up to the parser to tell them apart.

@wyli
Copy link
Contributor Author

wyli commented Sep 6, 2023

Follow-up of our previous discussions, the syntax change from # to :: seems to be technically feasible, but it'll affect many related codebases, FL, auto3dseg, model-zoo...

One way to make it backward compatible is to provide a system environment variable #6952 (by default it's non-breaking)

I think we should have another discussion about how to proceed.
cc @ericspod @Nic-Ma @KumoLiu

Please see a list of related code to this syntax change:

monai/auto3dseg/analyzer.py:100:            nested_key: str that has format of 'key1#0#key2'.
monai/auto3dseg/analyzer.py:105:            raise ValueError("Nested_key input format is wrong. Please ensure it is like key1#0#key2")
monai/fl/client/monai_algo.py:78:    if "validate#handlers" in parser:
monai/fl/client/monai_algo.py:79:        for h in parser["validate#handlers"]:
monai/bundle/config_parser.py:144:            id: id of the ``ConfigItem``, ``"#"`` in id are interpreted as special characters to
monai/bundle/config_parser.py:147:                For example: ``"xform#5"``, ``"net#channels"``. ``""`` indicates the entire ``self.config``.
monai/bundle/config_parser.py:170:            id: id of the ``ConfigItem``, ``"#"`` in id are interpreted as special characters to
monai/bundle/config_parser.py:173:                For example: ``"xform#5"``, ``"net#channels"``. ``""`` indicates the entire ``self.config``.
monai/bundle/config_parser.py:230:        For example, ``parser.update({"train#epoch": 100, "train#lr": 0.02})``
monai/bundle/config_parser.py:275:            id: id of the ``ConfigItem``, ``"#"`` in id are interpreted as special characters to
monai/bundle/config_parser.py:278:                For example: ``"xform#5"``, ``"net#channels"``. ``""`` indicates the entire ``self.config``.
monai/bundle/config_parser.py:326:        `@##A` means `A` in the upper level. and replace the macro tokens with target content,
monai/bundle/config_parser.py:328:        ``"%default_net"``, ``"%/data/config.json#net"``.
monai/bundle/config_parser.py:333:            id: id of the ``ConfigItem``, ``"#"`` in id are interpreted as special characters to
monai/bundle/config_parser.py:336:                For example: ``"xform#5"``, ``"net#channels"``. ``""`` indicates the entire ``self.config``.
monai/bundle/config_parser.py:355:        `@##A` means `A` in the upper level. and replace the macro tokens with target content,
monai/bundle/config_parser.py:357:        ``"%default_net"``, ``"%/data/config.json#net"``.
monai/bundle/config_parser.py:368:            id: id of the ``ConfigItem``, ``"#"`` in id are interpreted as special characters to
monai/bundle/config_parser.py:371:                For example: ``"xform#5"``, ``"net#channels"``. ``""`` indicates the entire ``self.config``.
monai/bundle/config_parser.py:413:        ``"#"`` in the config keys are interpreted as special characters to go one level
monai/bundle/config_parser.py:454:        The file path should end with `(json|yaml|yml)`. The component id should be separated by `#` if it exists.
monai/bundle/config_parser.py:472:        relative ID name which starts with the `ID_SEP_KEY`, for example, "@#A" means `A` in the same level,
monai/bundle/config_parser.py:473:        `@##A` means `A` in the upper level.
monai/bundle/config_parser.py:480:                "B": {"key": "@##A", "value1": 2, "value2": "%#value1", "value3": [3, 4, "@#1"]},
monai/bundle/config_parser.py:483:        It will resolve `B` to `{"key": "@A", "value1": 2, "value2": "%B#value1", "value3": [3, 4, "@B#value3#1"]}`.
monai/bundle/config_item.py:244:            return target  # for feature discussed in project-monai/monai#5852
monai/bundle/utils.py:27:ID_SEP_KEY = "#"  # separator for the ID of a ConfigItem
monai/bundle/utils.py:101:    "trainer": {"id": "train#trainer", "handlers": "train#handlers"},
monai/bundle/utils.py:102:    "validator": {"id": "validate#evaluator", "handlers": "validate#handlers"},
monai/bundle/workflows.py:202:            e.g. ``--net#input_chns 42``, ``--net %/data/other.json#net_arg``
monai/bundle/scripts.py:482:            _net_override = {f"network_def#{key}": value for key, value in net_override.items()}
monai/bundle/scripts.py:687:        python -m monai.bundle run --net#input_chns 1 ...
monai/bundle/scripts.py:693:        python -m monai.bundle run --net %/data/other.json#net_arg ...
monai/bundle/scripts.py:723:            e.g. ``--net#input_chns 42``, ``--net %/data/other.json#net_arg``.
monai/bundle/scripts.py:834:def _get_net_io_info(parser: ConfigParser | None = None, prefix: str = "_meta_#network_data_format") -> tuple:
monai/bundle/scripts.py:840:        prefix: a prefix for the input and output ID, which will be combined as `prefix#inputs` and
monai/bundle/scripts.py:841:            `prefix#outputs` to parse the input and output information in the `metadata.json` file of
monai/bundle/scripts.py:842:            a bundle, default to `meta_#network_data_format`.
monai/bundle/scripts.py:854:    prefix_key = f"{prefix}#inputs"
monai/bundle/scripts.py:855:    key = f"{prefix_key}#image#num_channels"
monai/bundle/scripts.py:857:    key = f"{prefix_key}#image#spatial_shape"
monai/bundle/scripts.py:859:    key = f"{prefix_key}#image#dtype"
monai/bundle/scripts.py:862:    prefix_key = f"{prefix}#outputs"
monai/bundle/scripts.py:863:    key = f"{prefix_key}#pred#num_channels"
monai/bundle/scripts.py:865:    key = f"{prefix_key}#pred#dtype"
monai/bundle/scripts.py:921:            e.g. ``--_meta#network_data_format#inputs#image#num_channels 3``.
monai/bundle/scripts.py:1091:            e.g. ``--_meta#network_data_format#inputs#image#num_channels 3``.
monai/bundle/scripts.py:1202:            e.g. ``--_meta#network_data_format#inputs#image#num_channels 3``.
monai/bundle/scripts.py:1346:            e.g. ``--_meta#network_data_format#inputs#image#num_channels 3``.
monai/bundle/reference_resolver.py:34:    the reference string may also contain a ``#`` character to refer to a substructure by
monai/bundle/reference_resolver.py:197:        The reference string starts with ``"@"``, like: ``"@XXX#YYY#ZZZ"``.
monai/bundle/reference_resolver.py:218:        The reference part starts with ``"@"``, like: ``"@XXX#YYY#ZZZ"``.
tests/test_config_parser.py:57:                {"_target_": "RandTorchVisiond", "keys": "@##0#keys", "name": "ColorJitter", "brightness": 0.25},
tests/test_config_parser.py:64:            "dataset": "@##dataset",
tests/test_config_parser.py:69:    ["transform", "transform#transforms#0", "transform#transforms#1", "dataset", "dataloader"],
tests/test_config_parser.py:106:        "C": "@#A",
tests/test_config_parser.py:107:        "D": {"key": "@##A", "value1": 2, "value2": "%#value1", "value3": [3, 4, "@#1", "$100 + @#0 + @##value1"]},
tests/test_config_parser.py:113:TEST_CASE_5 = [{"training": {"A": 1, "A_B": 2}, "total": "$@training#A + @training#A_B + 1"}, 4]
tests/test_config_parser.py:139:        parser["dataset#_target_"] = "Dataset"
tests/test_config_parser.py:140:        self.assertEqual(parser["dataset#_target_"], "Dataset")
tests/test_config_parser.py:141:        parser.update({"dataset#_target_1": "Dataset1"})
tests/test_config_parser.py:142:        self.assertEqual(parser["dataset#_target_1"], "Dataset1")
tests/test_config_parser.py:154:        trans = parser.get_parsed_content(id="transform#transforms#0")
tests/test_config_parser.py:157:        self.assertEqual(trans, parser.get_parsed_content(id="transform#transforms#0"))
tests/test_config_parser.py:158:        self.assertEqual(trans, parser.get_parsed_content(id="transform#transforms#0", lazy=True))
tests/test_config_parser.py:159:        self.assertNotEqual(trans, parser.get_parsed_content(id="transform#transforms#0", lazy=False))
tests/test_config_parser.py:161:        parser.set("fake_key", "transform#other_transforms#keys", True)
tests/test_config_parser.py:162:        self.assertEqual(parser.get(id="transform#other_transforms#keys"), "fake_key")
tests/test_config_parser.py:166:        parser["transform#transforms#0#keys"] = "label2"
tests/test_config_parser.py:167:        self.assertEqual(parser.get_parsed_content(id="transform#transforms#0").keys[0], "label2")
tests/test_config_parser.py:184:                parser[f"{id}#_mode_"] = "partial"
tests/test_config_parser.py:208:            config = {"A": {"B": 1, "C": 2}, "D": [3, "%A#B", "%#0", f"%{another_file}#E"]}
tests/test_config_parser.py:244:        np.testing.assert_allclose(parser.get_parsed_content("training#1", lazy=True), [0.7942, 1.5885], atol=1e-4)
tests/test_config_parser.py:266:            self.assertFalse("array#2" in parser)
tests/test_config_parser.py:349:            self.assertEqual(parser.get_parsed_content("key#unique"), expected_unique_val)
tests/test_config_parser.py:350:            self.assertIn(parser.get_parsed_content("key#duplicate"), expected_duplicate_vals)
tests/test_auto3dseg_ensemble.py:79:    "algo_spec_params": {"segresnet": {"network#init_filters": 8}, "swinunetr": {"network#feature_size": 12}},
tests/test_auto3dseg_ensemble.py:170:                _train_param["network#init_filters"] = 8
tests/test_auto3dseg_ensemble.py:173:                _train_param["network#feature_size"] = 12
tests/test_reference_resolver.py:32:        "transform#1": {"_target_": "LoadImaged", "keys": ["image"]},
tests/test_reference_resolver.py:33:        "transform#1#_target_": "LoadImaged",
tests/test_reference_resolver.py:34:        "transform#1#keys": ["image"],
tests/test_reference_resolver.py:35:        "transform#1#keys#0": "image",
tests/test_reference_resolver.py:37:    "transform#1",
tests/test_reference_resolver.py:46:        "dataloader#_target_": "DataLoader",
tests/test_reference_resolver.py:47:        "dataloader#dataset": "@dataset",
tests/test_reference_resolver.py:48:        "dataloader#collate_fn": "$monai.data.list_data_collate",
tests/test_reference_resolver.py:49:        "dataset#_target_": "Dataset",
tests/test_reference_resolver.py:50:        "dataset#data": [1, 2],
tests/test_reference_resolver.py:51:        "dataset#data#0": 1,
tests/test_reference_resolver.py:52:        "dataset#data#1": 2,
tests/test_reference_resolver.py:61:        "transform#1": {"_target_": "RandTorchVisiond", "keys": "image", "name": "ColorJitter", "brightness": 0.25},
tests/test_reference_resolver.py:62:        "transform#1#_target_": "RandTorchVisiond",
tests/test_reference_resolver.py:63:        "transform#1#keys": "image",
tests/test_reference_resolver.py:64:        "transform#1#name": "ColorJitter",
tests/test_reference_resolver.py:65:        "transform#1#brightness": 0.25,
tests/test_reference_resolver.py:67:    "transform#1",
tests/test_bundle_verify_net.py:40:            cmd += ["--device", "cpu", "--_meta_#network_data_format#inputs#image#spatial_shape", "[16,'*','2**p*n']"]
tests/test_bundle_verify_net.py:53:            cmd += ["--device", "cuda", "--_meta_#network_data_format#inputs#image#spatial_shape", "[16,'*','2**p*n']"]
tests/test_bundle_verify_net.py:54:            cmd += ["--_meta_#network_data_format#inputs#image#dtype", "float16"]
tests/test_bundle_verify_net.py:55:            cmd += ["--_meta_#network_data_format#outputs#pred#dtype", "float16"]
tests/test_bundle_verify_net.py:68:            **{"network_def#_target_": "tests.testing_data.bundle_test_network.TestMultiInputUNet"},
tests/testing_data/multi_gpu_evaluate.json:10:    "validate#sampler": {
tests/testing_data/multi_gpu_evaluate.json:12:        "dataset": "@validate#dataset",
tests/testing_data/multi_gpu_evaluate.json:16:    "validate#dataloader#sampler": "@validate#sampler",
tests/testing_data/multi_gpu_evaluate.json:23:        "$@validate#evaluator.logger.setLevel(logging.WARNING if dist.get_rank() > 0 else logging.INFO)"
tests/testing_data/multi_gpu_evaluate.json:26:        "$@validate#evaluator.run()"
tests/testing_data/multi_gpu_train.json:10:    "train#sampler": {
tests/testing_data/multi_gpu_train.json:12:        "dataset": "@train#dataset",
tests/testing_data/multi_gpu_train.json:16:    "train#dataloader#sampler": "@train#sampler",
tests/testing_data/multi_gpu_train.json:17:    "train#dataloader#shuffle": false,
tests/testing_data/multi_gpu_train.json:25:        "$@train#trainer.logger.setLevel(logging.WARNING if dist.get_rank() > 0 else logging.INFO)",
tests/testing_data/multi_gpu_train.json:26:        "$@validate#evaluator.logger.setLevel(logging.WARNING if dist.get_rank() > 0 else logging.INFO)"
tests/testing_data/multi_gpu_train.json:29:        "$@train#trainer.run()"
tests/testing_data/config_fl_stats_1.json:20:            "transform": "@train#preprocessing"
tests/testing_data/inference.json:56:        "data": "@_meta_#datalist",
tests/testing_data/config_fl_train.json:74:            "transforms": "$@train#training_transforms"
tests/testing_data/config_fl_train.json:88:            "transform": "@train#preprocessing"
tests/testing_data/config_fl_train.json:92:            "dataset": "@train#dataset",
tests/testing_data/config_fl_train.json:103:                "validator": "@validate#evaluator",
tests/testing_data/config_fl_train.json:117:            "train_data_loader": "@train#dataloader",
tests/testing_data/config_fl_train.json:121:            "inferer": "@train#inferer",
tests/testing_data/config_fl_train.json:122:            "train_handlers": "@train#handlers"
tests/testing_data/config_fl_train.json:162:            "transform": "@validate#preprocessing"
tests/testing_data/config_fl_train.json:166:            "dataset": "@validate#dataset",
tests/testing_data/config_fl_train.json:199:            "val_data_loader": "@validate#dataloader",
tests/testing_data/config_fl_train.json:201:            "inferer": "@validate#inferer",
tests/testing_data/config_fl_train.json:202:            "val_handlers": "@validate#handlers",
tests/testing_data/config_fl_train.json:203:            "postprocessing": "@validate#postprocessing",
tests/testing_data/config_fl_train.json:204:            "key_val_metric": "@validate#key_metric"
tests/testing_data/config_fl_train.json:211:        "$@train#trainer.run()"
tests/testing_data/config_fl_stats_2.json:20:            "transform": "@train#preprocessing"
tests/testing_data/config_fl_stats_2.json:36:            "transform": "@train#preprocessing"
tests/testing_data/inference.yaml:41:  data: "@_meta_#datalist"
tests/testing_data/config_fl_evaluate.json:2:    "validate#handlers": [
tests/testing_data/config_fl_evaluate.json:16:        "$@validate#evaluator.run()"
tests/test_fl_monai_algo_dist.py:46:        train_workflow.add_property(name="loader", required=True, config_id="train#training_transforms#0", desc="NA")
tests/test_integration_bundle_run.py:202:            override = "--network $@network_def.to(@device) --dataset#_target_ Dataset"
tests/test_integration_bundle_run.py:204:            override = f"--network %{overridefile1}#move_net --dataset#_target_ %{overridefile2}"
tests/test_integration_bundle_run.py:207:        cmd = "-m monai.bundle run --postprocessing#transforms#2#output_postfix seg"
tests/test_integration_bundle_run.py:219:        cmd = "-m fire monai.bundle.scripts run --tracking mlflow --evaluator#amp False"
tests/test_integration_autorunner.py:179:            "training#num_epochs_per_validation": train_param["num_epochs_per_validation"],
tests/test_integration_autorunner.py:180:            "training#num_images_per_batch": train_param["num_images_per_batch"],
tests/test_integration_autorunner.py:181:            "training#num_epochs": train_param["num_epochs"],
tests/test_integration_autorunner.py:182:            "training#num_warmup_epochs": train_param["num_warmup_epochs"],
tests/test_integration_autorunner.py:183:            "searching#num_epochs_per_validation": train_param["num_epochs_per_validation"],
tests/test_integration_autorunner.py:184:            "searching#num_images_per_batch": train_param["num_images_per_batch"],
tests/test_integration_autorunner.py:185:            "searching#num_epochs": train_param["num_epochs"],
tests/test_integration_autorunner.py:186:            "searching#num_warmup_epochs": train_param["num_warmup_epochs"],
tests/test_bundle_workflow.py:91:            "dataset#_target_": "Dataset",
tests/test_bundle_workflow.py:92:            "dataset#data": [{"image": self.filename}],
tests/test_bundle_workflow.py:93:            "postprocessing#transforms#2#output_postfix": "seg",
tests/test_bundle_workflow.py:122:            pairs={"validate#evaluator#postprocessing": "$@validate#postprocessing if @val_interval > 0 else None"}
docs/source/config_syntax.md:79:"@preprocessing#transforms#keys"
docs/source/config_syntax.md:82:_Description:_ `@` character indicates a reference to another configuration value defined at `preprocessing#transforms#keys`.
docs/source/config_syntax.md:83:where `#` indicates a sub-structure of this configuration file.
docs/source/config_syntax.md:86:"@preprocessing#1"
docs/source/config_syntax.md:91:Relative reference is supported by starting the reference with `#`. For example, `@#A` is to use `A` at the
docs/source/config_syntax.md:92:same config structure level, and `@##A` refers to `A` at one level above.
docs/source/config_syntax.md:125:"%demo_config.json#demo_net#in_channels"
docs/source/config_syntax.md:128:_Description:_ `%` character indicates a macro to replace the current configuration element with the texts at `demo_net#in_channels` in the
docs/source/config_syntax.md:206:- As "#" and "$" might be interpreted differently by the `shell` or `CLI` tools, may need to add escape characters
docs/source/config_syntax.md:207:  or quotes for them in the command line, like: `"\$torch.device('cuda:1')"`, `"'train_part#trainer'"`.

@ericspod
Copy link
Member

ericspod commented Sep 6, 2023

I'd suggest that we make :: a synonym for # and use both but recommend only the former.

@wyli
Copy link
Contributor Author

wyli commented Sep 6, 2023

I'd suggest that we make :: a synonym for # and use both but recommend only the former.

it's also feasible to use :: as the primary id anchor and we can add an id normalisation step to replace any # with ::, #6955 shows the idea (I can take more time to polish this if we go in this direction)

@wyli wyli closed this as completed in #6955 Sep 8, 2023
wyli added a commit that referenced this issue Sep 8, 2023
fixes #6721

### Description

compatible syntax by normalising any `#` in ids to `::`

```py
from monai.bundle import ConfigParser

config = {
    "my_dims": 2,
    "dims_1": "$@my_dims + 1",
    "my_net": {"_target_": "BasicUNet", "spatial_dims": "@dims_1", "in_channels": 1, "out_channels": 4},
}
# in the example $@my_dims + 1 is an expression, which adds 1 to the value of @my_dims
parser = ConfigParser(config)
print(parser.get_parsed_content("my_net::spatial_dims"))  # returns 3
print(parser.get_parsed_content("my_net#spatial_dims"))  # returns 3

```

new test cases:

https://github.com/Project-MONAI/MONAI/blob/66b50fb8384ae2dc3c76b39de673ce76908f94f2/tests/test_config_parser.py#L317-L321

### Types of changes
<!--- Put an `x` in all the boxes that apply, and remove the not
applicable items -->
- [x] Non-breaking change (fix or new feature that would not break
existing functionality).
- [ ] Breaking change (fix or new feature that would cause existing
functionality to change).
- [x] New tests added to cover the changes.
- [x] Integration tests passed locally by running `./runtests.sh -f -u
--net --coverage`.
- [x] Quick tests passed locally by running `./runtests.sh --quick
--unittests --disttests`.
- [x] In-line docstrings updated.
- [x] Documentation updated, tested `make html` command in the `docs/`
folder.

---------

Signed-off-by: Wenqi Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants