Meta tensor channel

[rijobro]

Description

Channel-based transforms to use MetaTensor.

Status

Ready

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.

[Nic-Ma]

Hi @rijobro ,

Thanks for your quick update for MetaTensor support.
I feel maybe we don't need to remove the NdarrayOrTensor support for the current stage? It doesn't block the development of MetaTensor in transforms so far. And we already completed NdarrayOrTensor support for all transforms.
If really necessary to deprecate NdarrayOrTensor, we can remove it later?
@wyli @ericspod What do you think?

Thanks.

[rijobro]

It's true that MetaTensor is a subclass of torch.Tensor, which is a sub-part of NdarrayOrTensor, so we could leave it. But the proposed changes mean that the input to our transforms will become torch.Tensor, and so I don't think it's bad that these PRs start implementing that. It enables us to remove some # type: ignore, and should improve readability.

I don't see the advantage of leaving them as NdarrayOrTensor, let me know your opinions on that one. Thanks

[wyli]

I think I agree with @Nic-Ma, perhaps removing NdarrayOrTensor (and removing the numpy support) could be done in a separate dedicated PR? our previous discussion here #3805 should be revisited...)

[rijobro]

Ok, how about this:

1. If a transform needs metadata (e.g., EnsureChannelFirstd), I will switch this to torch.Tensor, since it now can't be used with a np.ndarray as the metadata is no longer present,
2. If no metadata is required (e.g.,AddChanneld), I will leave this as NdarrayOrTensor.

Does that sounds reasonable?

[wyli]

sounds good to me, what do you think @Nic-Ma ?

[Nic-Ma]

Hi @rijobro ,

Plan sounds good to me!
Thanks for the discussion.

[Nic-Ma]

Overall looks good to me except for some minor comments inline.

Thanks.

[Nic-Ma]

Hi @rijobro ,

As this PR is going to merge, could you please help add the doc-string for args of update_meta:
https://github.com/Project-MONAI/MONAI/blob/feature/MetaTensor/monai/data/meta_tensor.py#L117
I got confused before checking the code details.

Thanks in advance.

[rijobro]

Hi @rijobro ,
As this PR is going to merge, could you please help add the doc-string for args of update_meta:
https://github.com/Project-MONAI/MONAI/blob/feature/MetaTensor/monai/data/meta_tensor.py#L117
I got confused before checking the code details.
Thanks in advance.

Sure @Nic-Ma, will do.

[rijobro]

@drbeh I needed to change this line from self.filter.to(prob_map) to self.filter.to(prob_map.device). Does this seem correct to you? Was the original code just to make sure the filter was on the same device as prob_map?

[bhashemian]

@drbeh I needed to change this line from self.filter.to(prob_map) to self.filter.to(prob_map.device). Does this seem correct to you? Was the original code just to make sure the filter was on the same device as prob_map?

@rijobro you are right but self.filter.to(prob_map) is equivalent of self.filter.to(prob_map.device).to(prob_map.dtype) and I'm not sure if we need the dtype here. There is no harm to keep it as it but I can double check if you really want to change it.

[wyli]

/black

[wyli]

mostly tested, merging it to trigger full integration tests.