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.

Sign up for GitHub

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

Update ['pixdim'] after Spacing transform in meta dict. #8269

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open

Update ['pixdim'] after Spacing transform in meta dict. #8269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
cdb8391
Fixes #6840
slicepaste Dec 23, 2024
8cc6175
fix issue 6840
slicepaste Dec 23, 2024
501bc45
fix issue 6840 format
slicepaste Dec 23, 2024
c5ebb0d
fix casting
slicepaste Dec 23, 2024
1aabdae
fix image_only flag issue
slicepaste Dec 23, 2024
fe05f59
fix format issue
slicepaste Dec 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions monai/data/image_reader.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -989,6 +989,8 @@ def get_data(self, img) -> tuple[np.ndarray, dict]:

for i, filename in zip(ensure_tuple(img), self.filenames):
header = self._get_meta_dict(i)
if MetaKeys.PIXDIM in header:
header[MetaKeys.ORIGINAL_PIXDIM] = np.array(header[MetaKeys.PIXDIM], copy=True)
header[MetaKeys.AFFINE] = self._get_affine(i)
header[MetaKeys.ORIGINAL_AFFINE] = self._get_affine(i)
header["as_closest_canonical"] = self.as_closest_canonical
Expand Down
4 changes: 4 additions & 0 deletions monai/data/meta_tensor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,10 @@ def pixdim(self):
return [affine_to_spacing(a) for a in self.affine]
return affine_to_spacing(self.affine)

def set_pixdim(self) -> None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense for this to be a method? If it's only going to be called in one place it's simple code could just be put there. If there's anticipation that this would be called by other things then that's fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your quick reply.
Originally, we considered other files such as DICOM might use Spacing, which could involve the usage and access meta_tensor.py property. Therefore, we decided to define a method.
However, after we reevaluating the entire codebase this week, it might be better to modify data["pixdim"] directly within TraceableTransform instead.

"""Update pixdim based on current affine."""
self.meta[MetaKeys.PIXDIM][1 : 1 + len(self.pixdim)] = affine_to_spacing(self.affine)

def peek_pending_shape(self):
"""
Get the currently expected spatial shape as if all the pending operations are executed.
Expand Down
3 changes: 3 additions & 0 deletions monai/transforms/spatial/array.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,9 @@ def __call__(
dtype=dtype,
lazy=lazy_,
)
if isinstance(data_array, MetaTensor) and "pixdim" in data_array.meta:
data_array = cast(MetaTensor, data_array.clone())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it necessary to clone the data array here? This is going to have a cost and I think isn't compatible with lazy resampling. Perhaps this is code that should be SpactialResample instead? @atbenmurray If you could please check if this is going to interact with laziness, thanks.

Copy link
Contributor Author

@slicepaste slicepaste Jan 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericspod Thank you for your suggestion.

We initially considered using data_array.clone() based on the following issue:

# LoadImage
data = {'image', 'data1.nii'}
imgloader = LoadImaged(keys=('image'), image_only=False, ensure_channel_first=True)
input_data_dict = imgloader(data)

# Spacing
respacing = transforms.Spacingd(keys=['image', 'label'], pixdim=(1, 1, 10), mode=('bilinear'))
spaced_data_dict = respacing(input_data_dict)

Originally, if we didn't use data_array.clone() and directly modified the data, the MetaTensor in both input_data_dict and spaced_data_dict would be affected simultaneously.
This means input_data_dict would lose its original input_data_dict["pixdim"] information.

However, as suggested above, using .clone() in this way is not ideal as it introduces additional costs.
If we perform this modification within TraceableTransform.track_transform_meta() method, which is executed by SpatialResample, it might no longer be a concern.

cc @einsyang723 @IamTingTing

data_array.set_pixdim()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that the pixel dimensions (pixdim) are only updated in the Spacing transformation. This is why I previously suggested that we should only retain the original_pixdim, as the latest pixdim can be derived from the metadata in the MetaTensor and the affine transformation.

Additionally, the use of meta_dict for logging metadata is becoming outdated. Perhaps we no longer need to maintain it? What are your thoughts on this?

import torch
from monai.transforms import Spacing

data = torch.randn(2, 1, 32, 32, 32)
trans = Spacing(pixdim=(1.5, 1.5, 1.0))
out = trans(data)
print(out.pixdim) -- > (1.5, 1.5, 1.0)

cc @ericspod @Nic-Ma

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@KumoLiu Thank you for your detailed feedback.

Our original thinking was that all related information should be update, including:

data['image']['pixdim']
data['image'].pixdim
data['image_meta_dict']['pixdim']

However, after reading your response, we're a bit uncertain: are you suggesting that we don't need to update pixdim in {key}_meta_dict?
Or will there be a new way to handle {key}_meta_dict in future releases?
If possible, could you please elaborate on this? Thank you.

cc @slicepaste @IamTingTing

if self.recompute_affine and isinstance(data_array, MetaTensor):
if lazy_:
raise NotImplementedError("recompute_affine is not supported with lazy evaluation.")
Expand Down
7 changes: 7 additions & 0 deletions monai/transforms/spatial/dictionary.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@
import numpy as np
import torch

import monai.transforms as transforms
from monai.config import DtypeLike, KeysCollection, SequenceStr
from monai.config.type_definitions import NdarrayOrTensor
from monai.data.box_utils import BoxMode, StandardMode
from monai.data.meta_obj import get_track_meta
from monai.data.meta_tensor import MetaTensor
from monai.data.utils import is_supported_format
from monai.networks.layers.simplelayers import GaussianFilter
from monai.transforms.croppad.array import CenterSpatialCrop
from monai.transforms.inverse import InvertibleTransform
Expand Down Expand Up @@ -520,6 +522,11 @@ def __call__(self, data: Mapping[Hashable, torch.Tensor], lazy: bool | None = No
output_spatial_shape=output_shape_k if should_match else None,
lazy=lazy_,
)
if isinstance(d[key], MetaTensor) and f"{key}_meta_dict" in d:
if "filename_or_obj" in d[key].meta and is_supported_format(
d[key].meta["filename_or_obj"], ["nii", "nii.gz"]
):
d = transforms.sync_meta_info(key, d)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I ask why we need this sync here seems it already been done in the MapTransform?

MONAI/monai/transforms/transform.py

Line 426 in 996e876

list_d[idx] = transforms.sync_meta_info(k, dict_i, t=not isinstance(self, transforms.InvertD))

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your question. In MapTransform, it only synchronizes MetaTensor, but since {key}_meta_dict is not a MetaTensor format, it won't be updated automatically. That's why we added this line of code to ensure the information in "{key}_meta_dict gets synchronized as well.

if output_shape_k is None:
output_shape_k = d[key].peek_pending_shape() if isinstance(d[key], MetaTensor) else d[key].shape[1:]
return d
Expand Down
2 changes: 2 additions & 0 deletions monai/utils/enums.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,8 @@ class MetaKeys(StrEnum):
Typical keys for MetaObj.meta
"""

PIXDIM = "pixdim" # MetaTensor.pixdim
ORIGINAL_PIXDIM = "original_pixdim" # the pixdim after image loading before any data processing
AFFINE = "affine" # MetaTensor.affine
ORIGINAL_AFFINE = "original_affine" # the affine after image loading before any data processing
SPATIAL_SHAPE = "spatial_shape" # optional key for the length in each spatial dimension
Expand Down
Loading