[Insight-developers] ImageSpatialObject Bug0006340

Stephen Aylward Stephen.Aylward at Kitware.com
Wed Aug 6 08:56:44 EDT 2008 

Set/GetDirection functions should not be in itkImage.
* For an itkImage, direction info should be stored in the meta data
dictionary - like modality type, etc.
* For an itkOrientedImage, those functions should exist.

However, there were reasons for this design.   The real problem is
that the oriented image implementation was never fully carried
through.   Two/three years later we're still paying the price.

If you want to patch this hole, probably the best solution is the following:

1) when you read into an image, the directions used by the get/set
directions functions are set to identity and the real direction
information is stored in the meta data dictionary.

2) On writing an image, the data in the meta data dictionary is
written as the directions in the file.   That way, we don't loose
direction information by reading/writing a file into an image.

The can be done fairly high in the IO framework so that it affects all
readers/writers - instead of changing the readers/writers
individually.

Ultimately, we should make all images in ITK oriented.   This would
break backward compatibility for people who have orientation
information in their files and have written custom filters that don't
use orientation (or use it via get/setdirection, or ...).
Regardless, we shouldn't try to keep backward compatibility in such
cases or we'll end up buried in confusing cmake variables.   We should
plan on this for release 4.0 when backward compatibility will be
broken for various other parts of the toolkit so that it can be
adapted to the evolving field.

Luis - are we going to start a branch to begin work on the 4.0 release?

Stephen

On Wed, Aug 6, 2008 at 8:24 AM, Rupert Brooks <rupe.brooks at gmail.com> wrote:
> I decided to investigate a bit further - I wondered if the problem
> could easily occur in practice.  I found that it can.
>
> I wrote a little code to load an image and print its direction info
> with both itk::Image, and itk::OrientedImage.  In both cases the
> direction info is correct, and non-identity.  To be clear - at least
> the MetaImageIO loads direction info and puts it into an ITK image,
> which would lead directly to an image that would create the problem i
> described.  So it would be very easy for a user to accidentally do
> this, leading to mysterious, occasional failures.
>
> I'm going to try changing itk::Image to always return unit direction
> cosines.  Dont worry, i wont submit a change that huge without
> discussing here first.  But I will let everyone know how many tests it
> breaks.  This will take a few hours, i'll report back when the build
> completes.
>
> By the way - I dont have particular feelings about a solution for this
> problem.  I use orientation in all my code, so the problem will never
> affect my work.  But i guess when you adopt a stray bug, like a stray
> puppy, you end up with all its consequences.   Feel free to just tell
> me what the solution should be - thats what I was hoping for in the
> first place anyway.
>
> Rupert
>
> On Tue, Aug 5, 2008 at 6:10 PM, Rupert Brooks <rupe.brooks at gmail.com> wrote:
>> Stephen,
>>
>> Yes, your example describes the problem perfectly.  There are no CMake
>> options that I'm aware of that would change it.  I think it happens in
>> every case.
>>
>> Your second comment brings up an important issue - aand a similar
>> problem has occurred.  The image derivatives on the oriented image
>> were previously not transformed by the image directions.  This lead to
>> problems and has been fixed, with a CMake switch.  However, the
>> SpatialObject and Image coordinate systems seem to be pretty much
>> separate code bases.   If derivatvies are taken relative to
>> SpatialObjects, then a similar problem *could* arise.  Perhaps it is
>> correct now, i havent checked.  Does anyone know?
>>
>> Still, i think this second point is a separate issue.  It seems the
>> design calls for the image IndexToPhysicalPoint transformation to
>> equate to the SpatialObject IndexToWorld transform - and thats not
>> causing problems.  Its just the unexpected existence of directions on
>> a itk::Image that does.
>>
>> Rupert
>>
>>
>> On Tue, Aug 5, 2008 at 4:28 PM, Stephen Aylward
>> <Stephen.Aylward at kitware.com> wrote:
>>> Hi Rupert,
>>>
>>> Am I understanding the problem correctly...
>>>
>>> ImageType::Pointer img = ImageType::New();
>>> img->SetDirections( someNonIdentityMatrix );
>>> img->TransformPhysicalPointToIndex(pnt, indx);
>>> std::cout << img->GetDirections() << std::endl;
>>>
>>> Will produce the exact same output (i.e., have the same directions) if
>>> ImageType is an itkImage or an itkOrientedImage, BUT the value of indx
>>> (which isn't printed) will potentially be very different.   That is,
>>> the images will have the same set of directions, but one will use it
>>> and one won't.
>>>
>>> We actually won't know what the code does unless we know what cmake
>>> options were set, right?
>>>
>>> Regretfully the concept of image direction was added after spatial
>>> objects were created.   ImageSpatialObjects (like many other spatial
>>> objects) have transforms which move from index space to object space
>>> and from object space to world space.   For imagespatialobjects, in
>>> the index space to object space transform, voxel spacing is
>>> considered.  For object space to world transform, the object (and its
>>> children-objects, e.g., extracted surfaces) are transformed to place
>>> them in the scene.
>>>
>>> So, the question becomes, should the directions in an image be applied
>>> to objects extracted from the image, or not?
>>>
>>> As specific examples, consider meshes or paths extracted from an image
>>> using one of the existing filters.   When you extract a mesh or a path
>>> from an image, in what space does it live: index space, object space,
>>> or world coordinates?   When considering this, it is particularly
>>> interesting to look at how  normals / gradients at node points are
>>> computed (e.g., how paths are steered as they are extracted from an
>>> image).  Worst case, if the image coordinates are transformed by the
>>> directions, but the gradients at those coordinates aren't specifically
>>> transformed by the directions, then those entities live in a hybrid
>>> space with some info (coordinates) in world space and some info
>>> (gradients) in object space.
>>>
>>> If we assume all is good, then we should apply the directions when
>>> going from index to object space, otherwise, the directions should be
>>> applied when going from object space to world space, so that the
>>> directions are applied to children objects as well.
>>>
>>> Stephen
>>>
>>> On Tue, Aug 5, 2008 at 3:31 PM, Bill Lorensen <bill.lorensen at gmail.com> wrote:
>>>> More questions:
>>>>
>>>> Can ImageSpatialObject be rewritten to use the Transform methods of
>>>> Image and OrientedImage?
>>>>
>>>> Bill
>>>>
>>>> On Tue, Aug 5, 2008 at 3:26 PM, Stephen Aylward
>>>> <Stephen.Aylward at kitware.com> wrote:
>>>>> So,
>>>>>
>>>>> Should an itkImage always return the identity matrix for its directions?
>>>>>
>>>>> or
>>>>>
>>>>> Should we allow inconsistency between recorded information and actual
>>>>> operation (record, but do not use direction in itkImage)?
>>>>>
>>>>> or
>>>>>
>>>>> Should we do-away with the itkImage without orientation, and should
>>>>> all images be itkOrientedImages?
>>>>>
>>>>>
>>>>> Stephen
>>>>>
>>>>>
>>>>>
>>>>> On Tue, Aug 5, 2008 at 3:19 PM, Rupert Brooks <rupe.brooks at gmail.com> wrote:
>>>>>> Hi Bill,
>>>>>>
>>>>>> Thats just the thing - the ImageSpatialObject does NOT use
>>>>>> TransformXtoY calls, because it has its own internal transformation
>>>>>> system.  It copies the information out of the image that it is given
>>>>>> and uses that as the spatial object IndexToWorld transform.  The bug
>>>>>> was caused because previously it did not copy the direction - this is
>>>>>> easily fixed.
>>>>>>
>>>>>> The tricky issue is that itk::Image always behaves as though its
>>>>>> direction is identity - but this is not enforced.  Its entirely
>>>>>> possible to put a non-identity direction in itk::Image.  This is not
>>>>>> used in the TransformXtoY calls of the image - but it is returned by
>>>>>> the GetDirection method.  So when i copy the direction information, if
>>>>>> there is some in the itk::Image, it breaks the test.
>>>>>>
>>>>>> As you can probably guess, i discovered that by accident.
>>>>>>
>>>>>> I agree that itk::Image should not behave differently inside and
>>>>>> outside a Spatial object.   As I see it, theres 4 options - I think
>>>>>> its a design decision which is why im bringing it up here.
>>>>>>
>>>>>> 1. Explicitly test for the itk::Image case inside itk::SpatialObject,
>>>>>> and ignore its directions
>>>>>>    This will run into problems with subclasses of itk::Image - and
>>>>>> not all subclasses can be treated the same way.  Which leads to #2
>>>>>>
>>>>>> 2. Explicitly test for the itk::OrientedImage case inside
>>>>>> itk::SpatialObject and only use the direction in that case
>>>>>>    This runs into the reverse problem, that if another subclass of
>>>>>> itk::Image with direction information is used, it will break when used
>>>>>> in the itk::ImageSpatialObject.    Perhaps I am biased against this
>>>>>> because i actually have such a class in my code.   No such class
>>>>>> currently exists in ITK, that i know of.
>>>>>>
>>>>>> 3. Consider this a conceptual bug in itk::Image, and force the
>>>>>> itk::Image GetDirection method to always return identity.
>>>>>>    I think this is the most correct answer, but i fear backward
>>>>>> compatibility consequences.
>>>>>>
>>>>>> 4. Ignore the issue, and assume/hope that users wont put direction
>>>>>> cosines in an itk::Image anyway.
>>>>>>    This is by far the easiest approach, but i don't like knowing a
>>>>>> potential bug exists and not fixing it.  These type of sins always
>>>>>> seem to come back to haunt me :-)
>>>>>>
>>>>>> Rupert
>>>>>>
>>>>>> On Tue, Aug 5, 2008 at 2:08 PM, Bill Lorensen <bill.lorensen at gmail.com> wrote:
>>>>>>> Rupert,
>>>>>>>
>>>>>>> In general, itkImage directions are ignored (assumed identity).
>>>>>>> itkOrientedImage directions are used. I have not looked at the
>>>>>>> ImageSpatialObject yet. If it uses Transform{X}To{Y} type calls, then
>>>>>>> Image and OrientedImage should behave properly. I don't think itkImage
>>>>>>> should behave differently inside and outside of itkImageSpatialObject.
>>>>>>>
>>>>>>> Bill
>>>>>>>
>>>>>>> On Tue, Aug 5, 2008 at 1:58 PM, Rupert Brooks <rupe.brooks at gmail.com> wrote:
>>>>>>>> Hi Everyone,
>>>>>>>>
>>>>>>>> As Luis wisely suggested i waited until after the 3.8 release to work
>>>>>>>> on this  :-)  I still need some big picture advice thoug
>>>>>>>>> Please note that the first action to take, even before experimenting
>>>>>>>>> with a fix, is to add a tests that illustrates the failure. E.g. to
>>>>>>>>> add a test that exercise the condition you have identified as
>>>>>>>>> problematic.
>>>>>>>>
>>>>>>>> I've added the test (itkImageMaskSpatialObjectTest2), and you all
>>>>>>>> should notice it failing in your dashboards shortly :-)
>>>>>>>>
>>>>>>>> Theres more than one way to patch the problem - i'll refer back to my
>>>>>>>> previous email.  I think theres a conceptual bug, or at least
>>>>>>>> confusion in the itk::Image. I cant decide how i should make
>>>>>>>> ImageSpatialObject behave when handed an ITK image that has
>>>>>>>> non-identity directions.
>>>>>>>>
>>>>>>>>>> Briefly, the image spatial object takes the coordinate transform from
>>>>>>>>>> the image that it is given, and uses that as its IndexToWorld
>>>>>>>>>> transformation.  Previously, it was ignoring the direction.  The fix
>>>>>>>>>> was a matter of adding the necessary lines to copy the direction
>>>>>>>>>> information also.
>>>>>>>>>>
>>>>>>>>>> However, this fix raises a further issue, and I'd like advice:
>>>>>>>>>>
>>>>>>>>>> The itk::Image class may have a non-identity direction component, but
>>>>>>>>>> it always ignores it.  However, a naive implementation of my fix would
>>>>>>>>>> use that direction information, giving a different behavior than the
>>>>>>>>>> itk::Image.  I could write code to check if its an ITK image, and then
>>>>>>>>>> behave differently, but this seems inelegant at best.   Is this
>>>>>>>>>> actually another bug - should an itk::Image always have an identity
>>>>>>>>>> direction, to conform with its behavior?  Can i ignore the issue,
>>>>>>>>>> assuming the user will be bright enough to only put direction
>>>>>>>>>> information in an itk::Image for a very good reason.
>>>>>>>>
>>>>>>>> Does anyone have some insights into how the itkImageSpatialObject
>>>>>>>> should behave when handed an itk::Image which has non-identity
>>>>>>>> direction cosines?
>>>>>>>>
>>>>>>>> (the bug in question is this one
>>>>>>>> http://public.kitware.com/Bug/view.php?id=0006340)
>>>>>>>>
>>>>>>>> Cheers
>>>>>>>> Rupert B.
>>>>>>>>
>>>>>>>>
>>>>>>>> --
>>>>>>>> --------------------------------------------------------------
>>>>>>>> Rupert Brooks
>>>>>>>> McGill Centre for Intelligent Machines (www.cim.mcgill.ca)
>>>>>>>> Ph.D Student, Electrical and Computer Engineering
>>>>>>>> http://www.cyberus.ca/~rbrooks
>>>>>>>> _______________________________________________
>>>>>>>> Insight-developers mailing list
>>>>>>>> Insight-developers at itk.org
>>>>>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> Insight-developers mailing list
>>>>>>> Insight-developers at itk.org
>>>>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> --------------------------------------------------------------
>>>>>> Rupert Brooks
>>>>>> McGill Centre for Intelligent Machines (www.cim.mcgill.ca)
>>>>>> Ph.D Student, Electrical and Computer Engineering
>>>>>> http://www.cyberus.ca/~rbrooks
>>>>>> _______________________________________________
>>>>>> Insight-developers mailing list
>>>>>> Insight-developers at itk.org
>>>>>> http://www.itk.org/mailman/listinfo/insight-developers
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Stephen R. Aylward, Ph.D.
>>>>> Chief Medical Scientist
>>>>> Kitware, Inc. - Chapel Hill Office
>>>>> http://www.kitware.com
>>>>> (518) 371-3971 x300
>>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Stephen R. Aylward, Ph.D.
>>> Chief Medical Scientist
>>> Kitware, Inc. - Chapel Hill Office
>>> http://www.kitware.com
>>> (518) 371-3971 x300
>>>
>>
>>
>>
>> --
>> --------------------------------------------------------------
>> Rupert Brooks
>> McGill Centre for Intelligent Machines (www.cim.mcgill.ca)
>> Ph.D Student, Electrical and Computer Engineering
>> http://www.cyberus.ca/~rbrooks
>>
>
>
>
> --
> --------------------------------------------------------------
> Rupert Brooks
> McGill Centre for Intelligent Machines (www.cim.mcgill.ca)
> Ph.D Student, Electrical and Computer Engineering
> http://www.cyberus.ca/~rbrooks
>



-- 
Stephen R. Aylward, Ph.D.
Chief Medical Scientist
Kitware, Inc. - Chapel Hill Office
http://www.kitware.com
(518) 371-3971 x300

More information about the Insight-developers mailing list