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

Resolve the confusion about dimension order and orientation through their respective keyword arguments in LoadImage #7365

Open
ibro45 opened this issue Jan 4, 2024 · 4 comments
Labels
Design discussions related to the generic API designs need discussions

Comments

@ibro45
Copy link
Contributor

ibro45 commented Jan 4, 2024

Is your feature request related to a problem? Please describe.

  • As a PyTorch user, I was surprised that MONAI does not follow PyTorch's dimension order convention (MONAI: WHD, PyTorch: DHW).

PyTorch 3D Conv docs:
image


  • I also thought that it is odd that the images are not represented in the way that a radiologist would look at them, with the patient facing up or down, instead of left or right. However, I understand that this is the convention that MONAI settled on. I will only be proposing a more transparent and flexible approach further below.
image
  • Finally, the reverse_indexing option in LoadImage is misleading - I tried it with NrrdReader, NibabelReader, and ITKReader, and it only worked with ITKReader. At least in theory for my case (not to clutter the post, I will upload the picture in the response).

Here are a few issues/discussions describing the confusion/differences/conventions:


Describe the solution you'd like
Similarly to how LoadImage has ensure_channel_first, it would be really transparent, flexible, and a non-breaking change to have:

  1. order/dim_order, set by default to WHD as per MONAI convention, but a user could change it to DWH or any other combination.
  2. orientation, can be set to RAS, LPS, etc. I am not sure right now if there is a consistent convention for different formats or if it varies. It could be implemented in a "non-breaking" way, similar to how LoadImage's reader is implemented, but it might be useful to have it consistent for all formats.
  3. [Optional] If 1. is implemented, reverse_indexing is probably unnecessary.

Additional context
Personally, even though I have been using MONAI for quite a while now, I always transpose the images so that: 1) they follow the PyTorch convention, 2) we see the axial slice (first dim after batch and channel dims) in the same way that a radiologist would look at it. I also orient the images LPS, but that's irrelevant rn.

The proposed approach may be particularly important for sharing models/MONAI Bundle - the dimension order or orientation used can be different between models and easily overlooked, rendering the weights almost useless if the data is not in the correct format. The proposed feature would make it more transparent as well as more easily documented by developers/sharers.

@ibro45
Copy link
Contributor Author

ibro45 commented Jan 4, 2024

The ITKReader with and without reverse_indexing. When True, the values ended up being really small.

reverse_indexing=False
image

reverse_indexing=True, low values
image

@KumoLiu
Copy link
Contributor

KumoLiu commented Jan 5, 2024

Hi @ibro45, thanks for the reporting, the dimension order, as here said may not be a problem.
And for the reverse_indexing result, I don't think it will change the intensity of the image, why does the pic you pasted show a large difference between two max and min intensities?

@ibro45
Copy link
Contributor Author

ibro45 commented Jan 5, 2024

as here #362 (comment) may not be a problem

Indeed, networks don't care about the order, but users do, and, judging by the list of issues/discussions above, they get confused at times (including me). That definitely has to do with all sorts of conventions out there, and that issue cannot be solved, but I believe that it can be transparent, more easily changed, and consistent across readers.

A LoadImage(order="WHD", orientation="RAS") would make it much simpler to know what's going on by default and to manipulate it as you wish. Currently, you would have to use Transpose and Orientation to make it look identical to when read with SimpleITK and turned into numpy. Don't get me wrong, these transforms definitely should exist, but a more transparent, flexible, and consistent LoadImage would be beneficial.

reverse_indexing feels awkward - it's simply not used by some readers, without any warning. With others, it works. Moreover, there are other inconsistencies between readers and orientation, @surajpaib will open a different issue.

For ease of use, I believe all MONAI readers should be consistent with regard to the order and the orientation of the images they load.

@ibro45
Copy link
Contributor Author

ibro45 commented Jan 13, 2024

Alright, so there's something we realized just now. We used to get it into the PyTorch convention by doing:

Orientation(axcodes="LPS")
Transpose(indices=[0, 3, 2, 1]) # cxyz -> czyx

And we used to have issues with Transpose not updating the metadata accordingly. After finding #5975, we figured out that we can just do Orientation with "SPL" instead of "LPS" and no Transpose to achieve the PyTorch convention:

Orientation(axcodes="SPL")

I think that, therefore, having orientation argument in LoadImage set to "RAS" by default with an explanation in the docstrings on how to manipulate it to achieve a certain convention would be really nice and understandable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design discussions related to the generic API designs need discussions
Projects
None yet
Development

No branches or pull requests

2 participants