Implement TorchIO transforms wrapper analogous to TorchVision transfo…

[SomeUserName1]

…rms wrapper and test case

Fixes #7499 .

Description

As discussed in the issue, this PR implements a wrapper class for TorchIO transforms, analogous to the TorchVision transforms wrapper.

The test cases just check that transforms are callable and that after applying a transform, the result is different from the inputs.

Types of changes

• 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).
• 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.
• In-line docstrings updated.
• Documentation updated, tested make html command in the docs/ folder.

[SomeUserName1]

Not sure where I have to add the torchio dependency to make CI pass.
All min-dep checks will fail I guess as i put the dependency in requirements-dev.txt, environment-dev.yml and setup.cfg.
The premerge-quick should pass.

[KumoLiu]

Hi @SomeUserName1, you can add a skip wrapper like this:

MONAI/tests/test_patch_wsi_dataset.py

Line 184 in c86e790

@skipUnless(has_cim, "Requires cucim")

Thanks.

[SomeUserName1]

One test failed, i have no idea why as the test and corresponding code is nothing that I've touched

Traceback (most recent call last):
  File "/home/runner/work/MONAI/MONAI/tests/test_regularization.py", line 73, in test_cutmixd
    self.assertTrue(not torch.allclose(output["a"], output["b"]))
AssertionError: False is not true

[KumoLiu]

Looks like a random error, fixed in this commit.
40bb130

[SomeUserName1]

@KumoLiu only the blossom-ci check hasn't passed. Can we merge?

[KumoLiu]

Hi @SomeUserName1, thanks for the PR! We might consider incorporating this new feature after the 1.3.1 release due to the new requirements included in this PR.

[johnzielke]

I found this by accident, so first of all: Really nice, I'd love to be able to easily use some of the transforms in torchio, and I think it helps the available transforms there gain visibility.

While looking at this, I had a few thoughts:

1. It would be nice to have this available for the dictionary versions as well (like torchvisiond)
2. The transform does currently not implement the RandomizableTrait, so I'm not sure how this would interact with CacheDatasets etc. I think it might help to have a version with the RandomizableTrait for all Random transforms (it seems like these are easy to find since they start with "Random") and one without for the rest. I'm not sure how to best design this, but one could maybe use the __new__() method to select the correct one based on the name. Alternatively an error/warning could be thrown if the wrong version is used.

[SomeUserName1]

1. It would be nice to have this available for the dictionary versions as well (like [torchvisiond](https://docs.monai.io/en/stable/transforms.html#torchvisiond))

Implemented.

2. The transform does currently not implement the RandomizableTrait, so I'm not sure how this would interact with CacheDatasets etc. I think it might help to have a version with the RandomizableTrait for all Random transforms (it seems like these are easy to find since they start with "Random") and one without for the rest. I'm not sure how to best design this, but one could maybe use the `__new__()` method to select the correct one based on the name. Alternatively an error/warning could be thrown if the wrong version is used.

They also have a common RandomTransform base class in tio, which differs from MONAIs base RandomTransform.
All tio transforms are probabilistic and support the p= parameter specifying the probability that the transform is applied. This is analogous to RandomizableTransform in MONAI.
The RandomTransform in tio states that there is randomness in the applied transform itself (like RandomNoise), additionally that its application (or not application) is probabilistic.

Thus, I add the RandomizableTrait as ABC to the TorchIO wrapper for all transforms.

I'm not sure if I should add Randomizable or RandomTranform as well as this is implemented in torchio already. So adding these two would be clearer in terms of the capabilities of these transforms but require manipulating TorchIO objects/class instances in MONAI code introducing complexity with little practical value (probabilistic application can be done via the tio parameters; is there a benefit in adding the same functionality via MONAI with the different concepts in mind?)

[johnzielke]

Thank you very much!
Regarding the p argument. One could potentially use the Monai RandomizableTransform and it's prob parameter, and then force p=1 to only use the monai code. The only benefit of this in my opinion is a slightly easier understandable interface on the MONAI side, and that the randomstate from Monai is used. But I'm not sure it's worth it / necessary. As a intermediate way, you could consider rerouting the "prob" parameter to "p" as well (ensuring only one of them is set at any time)

[KumoLiu]

Sorry for the delayed review; the point release is finally done. I have some concerns about the support for randomness, leave the comments inline.

@ericspod could you please also help review the PR? Thanks!

[KumoLiu]

Hi @SomeUserName1, apologies for overlooking this PR. I’ll review it and aim to have it merged by the end of the week. Thanks.

[ericspod]

Is this something that you'd like to proceed with any time soon? otherwise I'd close the PR

Hi @SomeUserName1 I think it still stands that there's an issue of randomness. We can't change whether a transform is considered random or not at runtime since it's type-based, so either we have a random and non-random version of the transforms or state in the docstring to use only randomised TorchIO transforms. If you're good with the latter we can merge things sooner after a little review.

[SomeUserName1]

No worries!

I agree that the randomness issue still stands and will implement the latter version.

[...] or state in the docstring to use only randomised TorchIO transforms.

[SomeUserName1]

So, I added the changes as discussed.
Additionally, I added RandTorchVision as well to actually unify the wrappers as discussed in PR #5569 , mentioned by @KumoLiu.

[SomeUserName1]

Locally the 4 unit test classes i wrote ran through, mypy and the formatters passed, so I hope this will be the last time, I'll kick off the pipeline.
If the pipeline runs green, please review.

Thanks for your time @ericspod & @KumoLiu !

[ericspod]

I think this is good now, thanks!

[KumoLiu]

/build