[Insight-developers] GetInverse is unusable -- patch attached

[Paul Koshevoy]

Hi Tom,

It's been 2 years since I left the SCI Institute at the University of 
Utah.  I have a full time job at Sorenson Media, and occasional 
consulting for SCI Institute CRCNS group, maintaining the ir-tools, 
ir-tweak, ir-mosaic, etc... that I've created for them years ago.  I 
can't bring myself to spend the time on writing a technical report to 
justify these simple changes, as well as coming up with test cases and 
sample datasets.  Patching ITK with each new release is so much easier 
-- it's just adding 5-10 to 5 ITK transforms.

What I am saying is that to contribute these changes to the ITK I have 
to get over a significant (artificial) barrier.  I don't feel right 
spending my own time on this, and I can't charge the time it would take 
me to do this to SCI/CRCNS because that is not a priority for them.

I have the latest patch, as well as the fully patched files here:
https://code.sci.utah.edu/svn/ImageReconstruction/trunk/code/itk-patch/
Their SSL cert expired last weekend, so you may need to add a security 
exception to be able to view the page.

    Pavel.


Tom Vercauteren wrote:
> Hi Pavel,
>
> You definitely also have my vote for having a usable GetInverse method in ITK.
>
> What is the current status of things? I found a bug entry here:
>   http://www.kwwidgets.org/Bug/bug_view_advanced_page.php?bug_id=7876
> and a wiki page here:
>   http://www.itk.org/Wiki/Proposals:InverseTransform
>
> However I didn't find the IJ paper you mentioned. Is it still under preparation?
>
> As far as technical matters are concerned, since I didn't find the IJ
> submission, I'll post some coments here.
>
> I think that having a virtual function that returns a
> itk::Transform<ScalarType,OutDim,InDim> is the way to go. The only
> thing that I don't quite like in the patch from the bug tracker is the
> fact that the typedef you use may be somewhat confusing for a user.
>
> Indeed at first glance it looks like one could change the following
> typedef to match the concrete transform type:
>    /** Base inverse transform type. */
>    typedef typename Superclass::InverseTransformType InverseTransformType;
>    typedef SmartPointer< InverseTransformType > InverseTransformPointer;
>
> Of course it would be wrong to do so as the base class function
> GetInverse wouldn't be overridden anymore. The only required change
> might be to either get rid of this typedef as in the above wiki page
>    virtual typename Transform<ScalarType, OutDim,InDim>::Pointer
> GetInverse() const;
>  or to change the naming of this typedef and enhance the
> documentation, e.g. something like:
>    /** Base inverse transform type. This type cannot be changed to the
> concrete transform type or inheritance would be lost. */
>    typedef typename Superclass::InverseTransformBaseType
> InverseTransformBaseType;
>    typedef InverseTransformBaseType::Pointer InverseTransformBasePointer;
>
> As a side note, for such an application, I believe that the best would
> be to use covariant return types
> (http://www.parashift.com/c++-faq-lite/virtual-functions.html#faq-20.8,
> http://en.wikipedia.org/wiki/Covariant_return_type). Alas this is not
> possible for two reasons:
>
> 1) DerivedTransformType::Pointer does not derive from
> TransformType::Pointer so I don't think covariant return types can be
> used. A raw pointer instead of a smart pointer would have to be used
> but this wouldn't fit in ITK standards since GetInverse allocates an
> object. A raw pointer could be returned if the inverse transformation
> was a member of the forward transformation but this doesn't sound
> quite nice.
>
> 2) No matter the first reason, ITK supports MSVC 6 but according to
> C++ FAQ lite, MSVC 6 doesn't support covariant return types.
>
> Regards,
> Tom
>
>
> On Fri, Oct 10, 2008 at 16:55, Pavel Koshevoy <koshevoy at sci.utah.edu> wrote:
>   
>> Hi,
>>
>> I've been patching ITK since version 1.8.1 to provide a usable GetInverse
>> API for transforms which don't have an analytic inverse, or have an
>> approximate inverse transform of a different type.
>>
>> Requiring a patched ITK is a hurdle in the path of developers trying to
>> build my image registration apps.  I really, really hope you merge this
>> patch into the trunk.
>> It's a very small change.  Basically, I define an InverseTransformPointer
>> type and add virtual InverseTransformPointer GetInverse() const method.  The
>> patch implements this API for itk::Transform, itk::IdentityTransform,
>> itk::MatrixOffsetTransformBase, itk::ScaleTransform,
>> itk::TranslationTransform.
>> These are useful changes that make the ITK Transform classes far more
>> useful. In my image registration apps I implement this API in my Legender
>> Polynomial Transform, a Radial Basis Function Transform, Triangle Mesh
>> Transform, Radial Distortion Transform, Cascaded Transform, and a generic
>> Newtons Method Numeric Inverse Transform.
>>
>> I really hope someone takes a look at this short patch and adds it to the
>> main ITK trunk.
>>
>> Thank you,
>>   Pavel.
>>