Does iterated composition work as expected?

[ebrahimebrahim]

I am just curious, if monai.transforms.Compose is used to construct a new transform out of other monai.transforms.Compose objects, then does it behave as expected? That is, should I expect

monai.transforms.Compose([
  monai.transforms.Compose([t1, t2]),
  monai.transforms.Compose([t3, t4]),
])

to behave exactly the same as

  monai.transforms.Compose([t1, t2, t3, t4])

in every way?

Will there be different caching behavior if, for example, t1, t2, t3 are deterministic while t4 is randomizable

[KumoLiu]

Hi @ebrahimebrahim, I don't understand what the purpose of your Compose in Compose is. CacheDataset will only cache deterministic transforms' results during training and Compose is inherited from Compose, it may have some issues when caching.

MONAI/monai/data/dataset.py

Lines 883 to 884 in baa17a8

	# execute all the deterministic transforms
	if isinstance(_transform, RandomizableTrait) or not isinstance(_transform, Transform):

MONAI/monai/transforms/compose.py

Line 40 in baa17a8

class Compose(Randomizable, InvertibleTransform):

Hope it can help you, thanks!

[ebrahimebrahim]

I guess you meant "Compose inherits Randomizable" so that answer my question: the caching behavior in my examples would indeed be different. One should then be careful with iterated composition.

Thanks for the help!

I don't understand what the purpose of your Compose in Compose is.

The example I showed is a bit silly, but it shows what you'd effectively end up with if you constructed transforms via Compose in some library code and then later combined those transforms in another Compose in some application code. The lesson here is: don't Compose until you're ready to make your Dataset and use your transform.

[dzenanz]

I was just bitten by Compose not being deterministic, thus breaking caching mechanism.

class Base(pl.LightningModule):
  self.common_transforms = Compose(
            [
                LoadImaged(keys=["image", "label"]),
                EnsureChannelFirstd(keys=["image", "label"]),
                NormalizeIntensityd(keys=["image"]),
                LabelFilterd(keys="label", ...),
            ]
        )

class VariantOne(Base):
  self.train_transforms = Compose([self.common_transforms, RandSpatialCropd(...), ...])

class VariantTwo(Base):
  self.train_transforms = Compose([self.common_transforms, RandSpatialCropd(...), ...])

The above is a lot slower (approx. 10x in my case) to train than this:

class VariantX(Base):
  self.train_transforms = Compose([*self.common_transforms.transforms, RandSpatialCropd(...), ...])

The difference is using *self.common_transforms.transforms instead of self.common_transforms.

[wyli]

currently Compose is hard-coded as a randomizable transform which is not cached... might be helpful to make it more flexible, if all the transforms being composed are non-random transforms, the compose transform could be cached.

[ebrahimebrahim]

That would be an improvement for sure

But it would still result in the non-intuitive behavior described in my original post. That's a bit more of a design challenge to address

[atbenmurray]

Hi folks. I've spent some time thinking about this during some refactoring earlier this year and I've opened issue #7130 with a corresponding draft PR #7131. I think there is a good opportunity here to both resolve this issue and also resolve other incongruencies that occur with nested Composes. Please take a look

[ebrahimebrahim]

Really appreciate that you are addressing this and I love your proposed approach and the reasoning for it