[Insight-developers] Recent changes to Transforms break ITK's API

Fri Jul 26 17:29:20 EDT 2013

"TransformFileWriter" string is found in 10 Slicer files.

Andras

From: insight-developers-bounces at itk.org
[mailto:insight-developers-bounces at itk.org] On Behalf Of Jean-Christophe
Fillion-Robin
Sent: July 26, 2013 9:38 PM
To: Bill Lorensen
Cc: Ghayoor, Ali; Johnson, Hans J; insight-developers at itk.org
Subject: Re: [Insight-developers] Recent changes to Transforms break ITK's
API

Regarding Slicer, couldn't find any occurrences of "TranformWriter" string
in the code base.

Jc

On Fri, Jul 26, 2013 at 3:25 PM, Bill Lorensen <bill.lorensen at gmail.com>
wrote:

I guess I can put an ITK version #ifdef guard around the code.

I don't want to be a PITA about this. But a non-sophisticated customer would
not have a clue how to change their code by looking at the compiler errors.

Is there something we could do with partial specialization? If the code uses
a bad type, we could put a warning pragma of runtime warning that tells the
user explicitly what to do.

Also, it would be great if the migration guide was up-to-date and kept
up-to-date.

On Fri, Jul 26, 2013 at 3:02 PM, Johnson, Hans J <hans-johnson at uiowa.edu>
wrote:

All,

Ali is working on a solution that works similar to the Image readers/writers
so that.

I would argue that the old code in the example is an example of what was
possible, but only in the most limited of uses.  

It is very unlikely that this code exists in end users applications because
transforms with floating point were not possible from the registration
framework, and could not have been used in any of the optimizers, or in
anything that used the Get/SetParameters or Get/SetFixedParameters API.
Additionally, the files written to disk would have been written and read as
double precision numbers.

In short, the only transforms that could exists in float are those that are
manually initialized, and then they could only be used for transforming
points.  All other update or optimization operations would have (and did)
give compiler errors.

====================================

Over the past year there has been much discussion of the need for being able
to do registrations in floating point (especially under the ITKv4
registration framework), and this looks like the only way that we (several
people have looked at this) could see for moving forward.

I welcome any suggestions on how to preserve backwards compatibility, but I
agree with Matt that the previous behavior was not correct, and I would
rather not continue to provide that wrong behavior.  In my mind this is an
example of: PREFER COMPILE TIME ERROR TO RUNTIME ERROR.

To make the old code work, you now would need to specify that you wish to
write the transform as float

TranformWriterTemplate<float>  and the code would now have the correct
behavior.

It is backwards compatible with double (the most common and previously
correct case) because TransformWriter is an alias to
TrasnformWriterTemplate<double>

Modules/IO/TransformBase/test/itkTransformFileWriterTemplateTest.cxx:
typedef itk::TransformFileWriterTemplate<double>      TransformWriterType;

Hans

From: Bill Lorensen <bill.lorensen at gmail.com>
Date: Friday, July 26, 2013 1:37 PM
To: Matt McCormick <matt.mccormick at kitware.com>
Cc: Ali Ghayoor <ali-ghayoor at uiowa.edu>, ITK <insight-developers at itk.org>,
Hans Johnson <hans-johnson at uiowa.edu>
Subject: Re: [Insight-developers] Recent changes to Transforms break ITK's
API

My main concern is that old code generates a compiler error that gives no
clue as to what is wrong.

What info can we provide to the customer to help fix this error. I see
nothing in the migration guide. In fact, that guide has not been updated
since last November.

Even a warning would be useful with explicit instructions on what to change.
My case was simple for me to fix because I knew something had changed in the
transforms and the code base was small. Pity thepoor customer with a large
investment in using ITK code.

On Fri, Jul 26, 2013 at 2:16 PM, Matt McCormick <matt.mccormick at kitware.com>
wrote:

Hi Ali,

If a float transform was saved as a double on disk, that could be
considered a bug and I do not think there is harm in fixing the
behavior.  We still should fix the compliation IMHO.

Thanks,
Matt

On Fri, Jul 26, 2013 at 1:37 PM, Ghayoor, Ali <ali-ghayoor at uiowa.edu> wrote:
> Hello All,
>
> As Bill Lorensen has proven in his example, the new changes to ITK, due to
> the "ENH: Support single precision registration" patch, break API of
> "itkTransformFileReader/Writer" filters.
>
> In attached report file, I have explained about the importance of new
> changes, and the current backward compatibility issues that they cause.
> Also, it is shown that the old functionality had a bug in it, and moving
> forward this bug should not be re-introduced.
>
> I really appreciate if you ITK gurus take a look at this report and tell
> your ideas about the new changes.
>
> Thank you,
> Ali
>
>
>
> From: Bill Lorensen <bill.lorensen at gmail.com>
> Date: Sunday, July 14, 2013 10:31 AM
> To: ITK <insight-developers at itk.org>
> Subject: [Insight-developers] Recent changes to Transforms break ITK's API
>
> Folks,
>
> When I compile the following code I get this compilation error:
>
>
/home/lorensen/ProjectsGIT/ITKGerrit/Modules/Remote/WikiExamples/IO/Transfor
mFileWriter.cxx:
> In function 'int main(int, char**)':
>
/home/lorensen/ProjectsGIT/ITKGerrit/Modules/Remote/WikiExamples/IO/Transfor
mFileWriter.cxx:20:
> error: no matching function for call to
>
'itk::TransformFileWriterTemplate<double>::SetInput(itk::SmartPointer<itk::R
igid2DTransform<float>
>>&)'
>
/home/lorensen/ProjectsGIT/ITKGerrit/Modules/IO/TransformBase/include/itkTra
nsformFileWriter.hxx:78:
> note: candidates are: void
> itk::TransformFileWriterTemplate<ScalarType>::SetInput(const
> itk::TransformBaseTemplate<TScalarType>*) [with ScalarType = double]
>
>
----------------------------------------------------------------------------
-----------------------
> #include "itkRigid2DTransform.h"
> #include "itkTransformFileWriter.h"
>
> int main(int argc, char *argv[])
> {
>   std::string fileName;
>   if(argc == 1) // No arguments were provided
>   {
>     fileName = "test.tfm";
>   }
>   else
>   {
>     fileName = argv[1];
>   }
>
>   typedef itk::Rigid2DTransform< float > TransformType;
>   TransformType::Pointer transform = TransformType::New();
>
>   itk::TransformFileWriter::Pointer writer =
> itk::TransformFileWriter::New();
>   writer->SetInput(transform);
>   writer->SetFileName(fileName);
>   writer->Update();
>
>   return EXIT_SUCCESS;
> }
>
>
>
> ________________________________
> Notice: This UI Health Care e-mail (including attachments) is covered by
the
> Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is
confidential
> and may be legally privileged.  If you are not the intended recipient, you
> are hereby notified that any retention, dissemination, distribution, or
> copying of this communication is strictly prohibited.  Please reply to the
> sender that you have received the message in error, then delete it.  Thank
> you.
> ________________________________

-- 
Unpaid intern in BillsBasement at noware dot com

  _____  

Notice: This UI Health Care e-mail (including attachments) is covered by the
Electronic Communications Privacy Act, 18 U.S.C. 2510-2521, is confidential
and may be legally privileged.  If you are not the intended recipient, you
are hereby notified that any retention, dissemination, distribution, or
copying of this communication is strictly prohibited.  Please reply to the
sender that you have received the message in error, then delete it.  Thank
you. 

  _____  

-- 
Unpaid intern in BillsBasement at noware dot com

_______________________________________________
Powered by www.kitware.com

Visit other Kitware open-source projects at
http://www.kitware.com/opensource/opensource.html

Kitware offers ITK Training Courses, for more information visit:
http://kitware.com/products/protraining.php

Please keep messages on-topic and check the ITK FAQ at:
http://www.itk.org/Wiki/ITK_FAQ

Follow this link to subscribe/unsubscribe:
http://www.itk.org/mailman/listinfo/insight-developers

-- 
+1 919 869 8849

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/pipermail/insight-developers/attachments/20130726/2eb890f9/attachment.htm>