MONAI icon indicating copy to clipboard operation
MONAI copied to clipboard

gh-7563: add config wrapper

Open johnzielke opened this issue 1 year ago • 2 comments

Fixes #7563 .

Description

This PR introduces a wrapper special keyword that can be used to wrap/decorate a existing model key. It is intended to be used with functions that follow the decorator API in python (i.e. expect the first positional argument of the function to be the instance/function to be decorated). This can be used to keep the hierarchy inside a config when using tools like torch.compile or wrapping other datasets/models.

This is still a draft, but I'd like to gather opinions on this feature and its implementation. Open questions (non-exhaustive):

  • name of the special keyword (e.g. _wrapper_, _wrap_, _decorator_)
  • How to enable/handle this experimental feature

This feature should be handled as experimental, meaning subject to change and removal in the future. This way we can gain some experience using it and change it without worrying about backwards compatibility.

Breaking changes

The only breaking change I see is that configs that used the special keyword in their configs before, would have this value be interpreted differently now. In the current state, with the feature flag disabled (the default), the config will warn about usage of this special key since it's meaning could change in the future.

Types of changes

  • [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.
  • [ ] Integration tests passed locally by running ./runtests.sh -f -u --net --coverage.
  • [ ] 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.

johnzielke avatar Apr 30 '24 18:04 johnzielke

Hi @johnzielke thanks for the contribution! This looks like an interesting addition with some clearly useful use cases. When you are done mark the PR as ready and we can consider it though I'd want to the feedback of a few other developers on this feature.

ericspod avatar May 07 '24 13:05 ericspod

Hey @ericspod, thank you. I fixed a few minor issues but I think it's ready for review/discussion now.

johnzielke avatar May 13 '24 23:05 johnzielke