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

Add option to set determinism with bundle configs #4942

Closed
holgerroth opened this issue Aug 19, 2022 · 8 comments
Closed

Add option to set determinism with bundle configs #4942

holgerroth opened this issue Aug 19, 2022 · 8 comments

Comments

@holgerroth
Copy link
Collaborator

holgerroth commented Aug 19, 2022

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.

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Aug 19, 2022

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.

@wyli
Copy link
Contributor

wyli commented Aug 19, 2022

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

@holgerroth
Copy link
Collaborator Author

holgerroth commented Aug 19, 2022 via email

@wyli
Copy link
Contributor

wyli commented Aug 22, 2022

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
Copy link
Contributor

wyli commented Aug 22, 2022

maybe one valid feature request is to create two types of permissions for the run_eval flag,

run_eval = os.environ.get("MONAI_EVAL_EXPR", "1") != "0"

  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.

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Aug 22, 2022

Hi @wyli ,

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

Thanks.

@holgerroth
Copy link
Collaborator Author

holgerroth commented Aug 22, 2022

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
  }

@Nic-Ma
Copy link
Contributor

Nic-Ma commented Aug 23, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

No branches or pull requests

7 participants