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

Fri Jul 26 15:42:37 EDT 2013

Bill,

After 3 months of trying to find away to do this, we are at a loss for a better way to do it.  If you can help find a better solution, we will gladly adopt it.

Hans

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

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<mailto: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<mailto:bill.lorensen at gmail.com>>
Date: Friday, July 26, 2013 1:37 PM
To: Matt McCormick <matt.mccormick at kitware.com<mailto:matt.mccormick at kitware.com>>
Cc: Ali Ghayoor <ali-ghayoor at uiowa.edu<mailto:ali-ghayoor at uiowa.edu>>, ITK <insight-developers at itk.org<mailto:insight-developers at itk.org>>, Hans Johnson <hans-johnson at uiowa.edu<mailto: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<mailto: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<mailto: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<mailto:bill.lorensen at gmail.com>>
> Date: Sunday, July 14, 2013 10:31 AM
> To: ITK <insight-developers at itk.org<mailto: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/TransformFileWriter.cxx:
> In function ‘int main(int, char**)’:
> /home/lorensen/ProjectsGIT/ITKGerrit/Modules/Remote/WikiExamples/IO/TransformFileWriter.cxx:20:
> error: no matching function for call to
> ‘itk::TransformFileWriterTemplate<double>::SetInput(itk::SmartPointer<itk::Rigid2DTransform<float>
>>&)’
> /home/lorensen/ProjectsGIT/ITKGerrit/Modules/IO/TransformBase/include/itkTransformFileWriter.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

________________________________
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.
________________________________
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/pipermail/insight-developers/attachments/20130726/9a03819e/attachment.htm>