TUM-DAML / seml

SEML: Slurm Experiment Management Library
Other
165 stars 29 forks source link

Add support to flatten dicts with integer keys #113

Closed heborras closed 1 year ago

heborras commented 1 year ago

Reference issue

None yet.

What does this implement/fix?

Hi, this is a bit of a strange bug, but the fix is quite simple.

The dictionaries created from reading in .yaml files can legally contain keys, which are not strings, e.g. integers. However, the current implementation of flatten only handles string keys: https://github.com/TUM-DAML/seml/blob/1ac4b06fcace4014dae5b9ea2d75cdc9c44daf81/seml/utils.py#L125

And thus seml will crash, when adding a config file, which contains integers as keys. With the following error:

(seml_pyt-tf) [hborras@ceg-octane sensitivity-metric]$ seml mlp-mapped_test add experiment_configs/MNIST/mlp_mapped_noise_experiment.yaml
Traceback (most recent call last):
  File "/home/hborras/.conda/envs/seml_pyt-tf/bin/seml", line 8, in <module> 
    sys.exit(main())
  File "/home/hborras/.conda/envs/seml_pyt-tf/lib/python3.9/site-packages/seml/main.py", line 298, in main
    f(**vars(command))
  File "/home/hborras/.conda/envs/seml_pyt-tf/lib/python3.9/site-packages/seml/add.py", line 114, in add_config_files
    add_config_file(db_collection_name, config_file, force_duplicates,
  File "/home/hborras/.conda/envs/seml_pyt-tf/lib/python3.9/site-packages/seml/add.py", line 160, in add_config_file
    configs = generate_configs(experiment_config, overwrite_params=overwrite_params)
  File "/home/hborras/.conda/envs/seml_pyt-tf/lib/python3.9/site-packages/seml/config.py", line 170, in generate_configs
    reserved, next_level = unpack_config(experiment_config)
  File "/home/hborras/.conda/envs/seml_pyt-tf/lib/python3.9/site-packages/seml/config.py", line 23, in unpack_config
    config = convert_parameter_collections(config)
  File "/home/hborras/.conda/envs/seml_pyt-tf/lib/python3.9/site-packages/seml/config.py", line 52, in convert_parameter_collections
    flattened_dict = flatten(input_config)
  File "/home/hborras/.conda/envs/seml_pyt-tf/lib/python3.9/site-packages/seml/utils.py", line 131, in flatten
    items.extend(flatten(v, new_key, sep=sep).items())
  File "/home/hborras/.conda/envs/seml_pyt-tf/lib/python3.9/site-packages/seml/utils.py", line 131, in flatten
    items.extend(flatten(v, new_key, sep=sep).items())
  File "/home/hborras/.conda/envs/seml_pyt-tf/lib/python3.9/site-packages/seml/utils.py", line 131, in flatten
    items.extend(flatten(v, new_key, sep=sep).items())
  [Previous line repeated 1 more time]
  File "/home/hborras/.conda/envs/seml_pyt-tf/lib/python3.9/site-packages/seml/utils.py", line 125, in flatten
    new_key = parent_key + sep + k if parent_key else k
TypeError: can only concatenate str (not "int") to str

To fix this issue the PR explicitly converts any key to a string.

Additional information

The config file, which I used and which contains integers as keys is attached here: mlp_mapped_noise_experiment.yaml.zip

This should reproduce the error above on the current master branch.

gasteigerjo commented 1 year ago

Is this the only place where a non-string key causes troubles?

heborras commented 1 year ago

Is this the only place where a non-string key causes troubles?

As far as I can tell this is the only place. I also tested this fix with non-string values at the root-level of the configuration and that worked just fine.