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

Anatomical orientation in MetaImages ignored in ITK #1017

Open
dzenanz opened this issue Jun 13, 2019 · 16 comments

Comments

Projects
None yet
4 participants
@dzenanz
Copy link
Member

commented Jun 13, 2019

Description

AnatomicalOrientation tag in MetaImages is ignored. LPS is always assumed.

Steps to Reproduce

Have a .mha image with AnatomicalOrientation tag. Change the orientation e.g. from RAS to RAI. The image should be flipped after this change, but it isn't.

Additional Information

Direction cosines are read, but AnatomicalOrientation is not read, it is always assumed to be LPS.

What should be done: read AnatomicalOrientation, and if it is different from LPS apply an appropriate correction matrix to direction cosines.

Submitted by: @agirault.

@dzenanz dzenanz added the type:Bug label Jun 13, 2019

@agirault

This comment has been minimized.

Copy link
Contributor

commented Jun 13, 2019

Also:

  • ensure to handle the case where the acronym is invalid in the reader
  • update the Writer which currently infers the anatomical orientation from the direction cosines, to instead always write LPS, since that is the coordinate system associated with the direction matrix
@issakomi

This comment has been minimized.

Copy link
Member

commented Jun 13, 2019

It is worrying. I tested in the past all possible orientations with MetaImages read/write and they looked correct for me. IMHO, the direction matrix is fully sufficient. I calculate orientation code (RAI, ASL, etc.) from the final image myself using spatial adapter (and have own functions too). Everything was fine and results are were consistent with anatomical orientation code in the file.
Even more - i fully trusted MetaImage format and used files as reference.
If there is a bug and orientation is broken (i don't think so) - it were epic fail and i'll have to notify customers and i don't think they will ever use my software or highly likely anything based on ITK.
Here are MetaImage files i used as reference, all orientations, with right/left marker burned in:
Orientation test images
If something is wrong with, it were catastrophic.

Edit

looked again and i think it is false alarm, correct orientation is written. There are matrix and orientation code, i guess that reader takes matrix, writer writes both. So if you manually change orientation code and makes it inconsistent with the matrix - you break files, but any format can be broken.

Edit 2

Direction cosines are read, but AnatomicalOrientation is not read, it is always assumed to be LPS.

Default coordinate system is LPS and this tag doesn't change it. Matrix is read and images get correct orientation. This orientation code is not intended to change default coordinate system. Identity matrix is LPS (RAI in ITK's terminology). Others are what spatial adapter returns, e.g. "TransformMatrix = 0 1 0 0 0 -1 -1 0 0" is ASL, there are 48 possible orientation codes and it is working very well, IMHO.
This is probably the reason for the issue - you have assumed this orientation code changes default coordinate system, no, it doesn't. If you want new functionality - to change default coordinate system - you must introduce new tag, otherwise backward compatibility will be hopeless broken.

What should be done: read AnatomicalOrientation, and if it is different from LPS apply an appropriate correction matrix to direction cosines.

update the Writer which currently infers the anatomical orientation from the direction cosines, to instead always write LPS, since that is the coordinate system associated with the direction matrix

Again, it is not default coordinate system, it is orientation code for current matrix.

Edit 3:

MetaIO Documentation

AnatomicalOrientation MET_STRING
Specify anatomic ordering of the axis. Use only [R|L] | [A|P] | [S|I] per axis. For example, if the
three letter code for (column index, row index, slice index is) ILP, then the origin is at the superior, 
right, anterior corner of the volume, and therefore the axes run from superior to inferior, from
right to left, from anterior to posterior.
@dzenanz

This comment has been minimized.

Copy link
Member Author

commented Jun 14, 2019

Current treatment of AnatomicalOrientation tag seems to be purely for human convenience. @agirault recently came to the conclusion it should not be so, after discussion with @aylward and @thewtex. Perhaps reasoning and history behind this could be given here?

@agirault

This comment has been minimized.

Copy link
Contributor

commented Jun 14, 2019

Thanks for the feedback @issakomi. This is a sensitive change so we will make sure to run our findings and suggestions by the ITK community on Discourse to find the best course of action and maintain backward compatibility.

Just for a summary:

  • the AnatomicalOrientation tag was created as independent information from the direction matrix and was meant to describe the coordinate system resulting from applying the transformation to IJK coordinates. It was added 15 years ago to ITK as a tag that would be read and written as metadata, which again was complementary to the direction matrix, not redundant.
  • 14 years ago, changes were made in ITK were this tag was not read anymore, and it was computed from the direction matrix when being written. This made the tag redundant information to the direction matrix, which was not the intended use according to its creator (simple example: it doesn't make sense to infer the anatomical orientation if the transformation is not axis-aligned)
  • Even though this was not the original design, this has been in ITK for 14 years. We're aware that changing it would have big repercussion and we need to respect that.
  • On top of this, we need to better define what convention ITK uses to even define anatomical orientation: while DICOM/NRRD describes the axis at the tip of the XYZ vectors (L means +X=R->L), MetaIO and ITKSnap currently describe the axis as the origin of those vectors (L means +X=L->R). This was discussed a while back here.
@aylward

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

@dzenanz

This comment has been minimized.

Copy link
Member Author

commented Jun 14, 2019

But does this refer to ijk or XYZ origin and vectors? If it refers to ijk (index space), then direction matrix is redundant. If it refers to XYZ (physical space), then another (possible) flip is required to reconcile this with ITK convention that identity matrix corresponds to LPS.

@issakomi

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

@agirault
Thank you for responce. But this issue is marked as "bug", there is no bug. You interpret the AnatomicalOrientation tag as something related to reference coordinate system, but it is not, this 3 letters code is orientation of matrix according to above definition.

#include <itkSpatialOrientation.h>

ImageType::Pointer image = reader->GetOutput();
itk::SpatialOrientationAdapter adapter;
const unsigned int orientation = static_cast<unsigned int>(
   adapter.FromDirectionCosines(image->GetDirection()));
switch (orientation)
{
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_RIP:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_LIP:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_RSP:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_LSP:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_RIA:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_LIA:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_RSA:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_LSA:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_IRP:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_ILP:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_SRP:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_SLP:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_IRA:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_ILA:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_SRA:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_SLA:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_RPI:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_LPI:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_RAI:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_LAI:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_RPS:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_LPS:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_RAS:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_LAS:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_PRI:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_PLI:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_ARI:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_ALI:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_PRS:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_PLS:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_ARS:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_ALS:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_IPR:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_SPR:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_IAR:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_SAR:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_IPL:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_SPL:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_IAL:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_SAL:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_PIR:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_PSR:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_AIR:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_ASR:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_PIL:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_PSL:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_AIL:
    break;
  case itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_ASL:
    break;
  default:
    break;
  }
@issakomi

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

@aylward
I have never seen definition for 3-letters orientation code in DICOM (not sure it does exist), coordinate system is defined here.
Thanks heaven it is same with ITK's coordinate system. But terminology may be different, usually DICOM's is named DICOM LPS, same with ITK's RAI, same with Nrrd's left-posterior-superior. But not the same - Slicer's RAS, Slicer has different coordinate system.

Difference in terminology is usage of from or to notation.

@aylward

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

@aylward

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

@issakomi

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

@aylward

I think it borderlines on being a bug in Slicer, if what you say is true

Slicer team, AFAIK, will not accept this as bug. They say

However different medical applications use different definitions of this 3D basis.
Most common are the following bases:
LPS (Left, Posterior, Superior) is used in DICOM images and by the ITK toolkit
RAS (Right, Anterior, Superior) is similar to LPS with the first two axes flipped and used by 3D Slicer
Both bases are equally useful and logical.
It is just necessary to know to which basis an image is referenced.
@aylward

This comment has been minimized.

Copy link
Member

commented Jun 14, 2019

@agirault

This comment has been minimized.

Copy link
Contributor

commented Jun 17, 2019

There are two issues being discussed here:

  1. lately, the difference between definitions of 3D basis. We seem to all agree that we need to account for:
  • where to look at on the axis to describe it ("from RAI" = "to LPS", like for ITK)
  • different coordinate systems (if we go with the "to" definition, Slicer is RAS, ITK is LPS)

For this issue, further discussions can be made to better describe this, as ITK/MetaIO is the only one using the "from" definition, and is prone to a lot of confusion as the last couple comments can confirm. And that's not new.

  1. originally, the issue of the anatomical orientation tag being redundant with the transform matrix, which doesn't fit with the initial design. This was already described in comments above, summarized again by @aylward :

The direction cosine matrix exists in a space. That space is defined by
the AnatomicalOrientation. That is, when moving in the physical x-direction
(or in i-index direction with an identity direction matrix), then you are
going toward L when in an LPS anatomic space. Changing the direction
matrix will never change the fact that you're in an LPS space, but it will
change how moving along the i-index direction maps to a movement in that
space. It tells you that the first i-index pixel you access in memory is
to the right of the second pixel.

We'll draft a well-documented post explaining the ins-and-outs of the issue, suggest a versioning update of the MetaIO format to restore this initial feature while maintaining backward-compatibility with how MetaIO has been used for the last 14 years, then share it in the ITK discourse forum to get feedback from the community.

@aylward

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

@issakomi

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

in my apps i heavily use itk::SpatialOrientation::ITK_COORDINATE_ORIENTATION_.... enum
and compatibility of ITK and DICOM space, it is great advantage. I don't understand what is your goal? Rename RAI to LPS? There is no standard for that and i've never seen 48 orientation codes like in ITK, here and there is wording "DICOM LPS" (not defined in DICOM) and "Slicer RAS", nothing else.
I also don't understand what is wrong with direction cosines in MetaImage and why should any additional transforms be applied?
I will be not able to update to newer version of ITK, sorry, i have no time to re-write applications, i better stay with the old version of ITK and will back-port critical bugs... Also i have no time to participate in discussions, for me it is too clear. You are from Kitware, it is your playground, but i leave the discussion. Good luck.

@aylward

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.