MONAI icon indicating copy to clipboard operation
MONAI copied to clipboard

Add option to set determinism with bundle configs

Open holgerroth opened this issue 1 year ago • 8 comments

Is your feature request related to a problem? Please describe. There should be a configuration option to enable deterministic training with monai bundle. Currently, a user has to add python code in the "training" portion of the config:

    "training": [
        "$monai.utils.set_determinism(seed=123)",
        "$setattr(torch.backends.cudnn, 'benchmark', True)",
        "$@train#trainer.run()"
    ]

This can be difficult to use in security-constrained environments such as in federated learning, where one might want to disable the python code execution in the bundle.

Describe the solution you'd like There should be an option to set the deterministic behavior using configuration style options, e.g., the below to achieve the same behavior as above:

  "determinism": {
    "random_seed": 123,
    "benchmark": True
  }

Describe alternatives you've considered MonaiAlgo for executing FL training does not run the "training" section in the config. Therefore the code there is not being executed. One can achieve determinism by using the "required" option for the SupervisedTrainer but this will not work if python code is disabled.

Additional context Especially important feature to support FL.

holgerroth avatar Aug 19 '22 14:08 holgerroth

Hi @holgerroth ,

Thanks for raising the question. Actually, I think it's hard to execute the set_determinism() function without python expression, unless we hard-code some logic in the bundle ConfigParser to call it internally, which we are trying to avoid. Now we don't suppose any config logic in the JSON, totally use the JSON content to define the whole logic. For the FL program, if we can't execute python expressions in the config, can we call set_determinism() somewhere in the FL program before parsing the bundle? @wyli What do you think?

Thanks.

Nic-Ma avatar Aug 19 '22 16:08 Nic-Ma

Yes, perhaps we have this hardcoded in monaialgo for FL, and later extending this to the primary bundle parser if we receive feature requests? cc @Nic-Ma

wyli avatar Aug 19 '22 16:08 wyli

I feel MonaiAlgo is the wrong place for handling this. Determinism is a general MONAI feature that is used by many bundles. Why not have some purpose logic for this in the config parser?


From: Wenqi Li @.> Sent: Friday, August 19, 2022 12:07:02 PM To: Project-MONAI/MONAI @.> Cc: Holger Roth @.>; Mention @.> Subject: Re: [Project-MONAI/MONAI] Add option to set determinism with bundle configs (Issue #4942)

Yes, perhaps we have this hardcoded in monaialgo for FL, and later extending this to the primary bundle parser if we receive feature requests? cc @Nic-Mahttps://github.com/Nic-Ma

— Reply to this email directly, view it on GitHubhttps://github.com/Project-MONAI/MONAI/issues/4942#issuecomment-1220845396, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABQDH4W73GSRO47OCSNJOX3VZ6WKNANCNFSM57A7BMCA. You are receiving this because you were mentioned.Message ID: @.***>

holgerroth avatar Aug 19 '22 16:08 holgerroth

currently the config parser connects the structured configs and the python-based components. It is quite generic and decoupled from the underlying machine learning APIs. So adding numpy/torch specific code logic to it doesn't make sense to me...

however, we can define some system default pre-set config json, and the user can choose to load it as system-wide predefined commands, and override these default keys in their own config files.

"rng_seed": null,
"deterministic": [
    "$monai.utils.set_determinism(seed=@rng_seed)",
    "..."
]

the pre-set configs could be hosted in monai's data storage and requires checksum matching before loading/running

wyli avatar Aug 22 '22 12:08 wyli

maybe one valid feature request is to create two types of permissions for the run_eval flag, https://github.com/Project-MONAI/MONAI/blob/1941283b1862e3f578ccfeb2bdfa864171125029/monai/bundle/config_item.py#L318

  1. whether to eval any user-provided command strings
  2. whether to eval a set of predefined command strings from trusted sources

so that when option 1 is disabled at a system level, the user can still have the flexibility to use the 2nd option to run some predefined commonly used commands.

wyli avatar Aug 22 '22 13:08 wyli

Hi @wyli ,

Interesting idea, need further step to gather some predefined expressions?

Thanks.

Nic-Ma avatar Aug 22 '22 15:08 Nic-Ma

currently the config parser connects the structured configs and the python-based components. It is quite generic and decoupled from the underlying machine learning APIs. So adding numpy/torch specific code logic to it doesn't make sense to me...

Exactly, adding something like this to the config would be independent of the underlying implementation. There's no dependency on numpy/pytorch:

  "determinism": {
    "random_seed": 123,
    "benchmark": True
  }

holgerroth avatar Aug 22 '22 20:08 holgerroth

Hi @holgerroth ,

Based on the latest comment from @wyli , I think he means a different proposal: "eval a set of predefined command strings from trusted sources". @wyli Or maybe I misunderstood your idea?

Thanks.

Nic-Ma avatar Aug 23 '22 05:08 Nic-Ma