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.

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

Enable global coordinates in spatial crop transforms #8206

Open
surajpaib opened this issue Nov 14, 2024 · 14 comments
Open

Enable global coordinates in spatial crop transforms #8206

surajpaib opened this issue Nov 14, 2024 · 14 comments
Labels
enhancement New feature or request Feature request Feedback welcomed welcome developer and community feedback (make comments or create new tickets if needed)

Comments

@surajpaib
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently, image coordinates are passed as roi center in the spatial crop transforms. It would be nice to have the ability to pass physical coordinates as all the information required to generate the image coordinates exists in the metadata. This would allow using coordinates without worrying about things like spacing changes.

Describe the solution you'd like
I've got an implementation lying around in a custom package but would like if I can use just MONAI moving forward
https://github.com/AIM-Harvard/foundation-cancer-image-biomarker/blob/e447a6f69f5240b658f1a6e874bdbd4f4f45df99/fmcib/preprocessing/seed_based_crop.py#L121

@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 14, 2024

Hi @surajpaib, thanks for the proposal!
Currently, in MONAI, ROI centers are defined in image coordinates, and all cropping operations are performed at the pixel level. Could you share a bit more about your use case—specifically, when you would have the center coordinates available in global (physical) coordinates?

The reason I ask is that while some of our cropping transforms do allow specifying roi_center, these centers are expected to be provided in image (pixel) coordinates. Understanding your specific needs could help us explore potential support for physical coordinate input if that's necessary for your workflows.

roi_center: Sequence[int] | int | NdarrayOrTensor | None = None,

@surajpaib
Copy link
Contributor Author

Thanks for your prompt response!

Use-cases I'm thinking of are mainly when I would want to crop based on objects such as a markup annotation from slicer -w hich would be in physical coordinates by default.

Even when using coordinates from objects like masks, I generally prefer using a global coordinate so that I wouldn't have to recalculate image coordinates in the case that I might want to change the resolution/spacing of the image. I was wondering if there might be interest to a larger audience with this functionality.

@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 14, 2024

Use-cases I'm thinking of are mainly when I would want to crop based on objects such as a markup annotation from slicer -w hich would be in physical coordinates by default

Does CropForegroundd meet your requirement? You can set the object as your source_key.

@surajpaib
Copy link
Contributor Author

I can definitely use a lot of the MONAI transforms with my mask object without any trouble. My usecase was specifically for when I have extracted physical coordinates from these mask objects and then want to use those points to crop out regions in the image - very similar to the slicer scenario I mentioned.

@KumoLiu KumoLiu added enhancement New feature or request Feature request Feedback welcomed welcome developer and community feedback (make comments or create new tickets if needed) labels Nov 15, 2024
@KumoLiu
Copy link
Contributor

KumoLiu commented Nov 15, 2024

A potential solution from today's meeting: a helper function to help convert the global coordinates to the image coordinate.

@surajpaib
Copy link
Contributor Author

@KumoLiu This sounds good to me. Where would this helper function go ideally? Somewhere like monai.transforms.utils?

Happy to send a PR then

@aylward
Copy link
Collaborator

aylward commented Nov 21, 2024

Perhaps the helper function should be a member function of an image? The image has the necessary and sufficient info.

@ericspod
Copy link
Member

ericspod commented Dec 2, 2024

Perhaps the helper function should be a member function of an image? The image has the necessary and sufficient info.

My thought process on this was to have a function wrapped as a dictionary transform which would be applied to a key containing the physical coordinates and transform them as needed. The ROI for subsequent transforms can be defined in terms of that key name rather than given values. This seems like a more modular way of doing things rather than adding more things to existing transforms. This could take advantage of ApplyTransformToPoints as well. An alternative is a transform used as a wrapper around another which manipulates argument values.

@aylward
Copy link
Collaborator

aylward commented Dec 4, 2024

Ok - I see your point. I agree, it should be a dictionary transform. What I'm a bit uncertain about is how do we specify which physical transform matrix is applied to the points. It would be best (imo) if the concept/scope of this dictionary transform was limited to transforming physical points into a specific image's coordinate frame (and not simply arbitrarily transformed). And we should have a parallel dictionary transform that can go from image coordinates back to physical coordinates. What do you see as the parameter list for those dictionary transforms?

@ericspod
Copy link
Member

ericspod commented Dec 5, 2024

Consider these transforms in a transform sequence with a dictionary containing ROI start and end coordinates under their own keys. We would use ApplyTransformToPointsd to convert these from global coordinates to coordinates in the image to crop, then the roi_start and roi_end arguments for SpatialCropd would be modified to accept key names rather than values. When this transform is applied the key names are used for lookup with the assumption they contain coordinates in image space. This requires modifying SpatialCropd but I think ApplyTransformToPointsd will work as it is.

[
...
ApplyTransformToPointsd(key="roi_start_key", refer_key="image_to_crop"),
ApplyTransformToPointsd(key="roi_end_key", refer_key="image_to_crop"),
SpatialCropd(key="image_to_crop",roi_start="roi_start_key", roi_end="roi_end_key"),
...
]

@aylward
Copy link
Collaborator

aylward commented Dec 11, 2024

Thanks for the clarification! Nice implementation.

One suggestion...perhaps we need three functions:

  1. ApplyTransformToPointsd() - should be exactly as-is and should serve as a base class for the next two

For the other two, they make it explicit what spaces we're moving the points between and in what directions. The question is the name for those two functions. Here are options:

Option A (my favorite, but that's because it is similar to ITK):
2) TransformSpatialPointsToIndexes()
3) TransformIndexesToSpatialPoints()

or Option B:
2) TransformGlobalPointsToLocalPoints()
3) TransformLocalPointsToGlobalPoints()

or Option C:
2) TransformGlobalPointsToObjectSpace()
3) TransformObjectPointsToGlobalSpace()

or ???

Aside from Images, are ther

@ericspod
Copy link
Member

This would be something like:

class TransformPointsWorldToImaged(ApplyTransformToPointsd):
    """[Sensible docstring here]"""
    def __init__(
        self,
        keys: KeysCollection,
        refer_keys: KeysCollection,
        dtype: DtypeLike | torch.dtype = torch.float64,
        affine_lps_to_ras: bool = False,
        allow_missing_keys: bool = False,
    ):
        super().__init__(keys, refer_keys, dtype, None, True, affine_lps_to_ras, allow_missing_keys)

class TransformPointsImageToWorldd(ApplyTransformToPointsd):
    """[Sensible docstring here]"""
    def __init__(
        self,
        keys: KeysCollection,
        refer_keys: KeysCollection,
        dtype: DtypeLike | torch.dtype = torch.float64,
        affine_lps_to_ras: bool = False,
        allow_missing_keys: bool = False,
    ):
        super().__init__(keys, refer_keys, dtype, None, False, affine_lps_to_ras, allow_missing_keys)

These vary by whether the transform applied is inverted or not. The refer_keys argument is also required now. The names still need work.

@aylward
Copy link
Collaborator

aylward commented Dec 16, 2024

Nice! Agreed!

TransformImageToWorldPointsd
TransformWorldToImagePointsd
?

@surajpaib
Copy link
Contributor Author

Hi all - sorry for being a bit checked out on this - was on a sabbatical.

The suggestions look great and would be perfect for the use-case I'd envisioned.

Also adding a suggestion, based on personal preference for the naming:
TransformImagetoPhysicalCoordinatesd/ TransformPhysicaltoImageCoordinatesd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Feature request Feedback welcomed welcome developer and community feedback (make comments or create new tickets if needed)
Projects
None yet
Development

No branches or pull requests

4 participants