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

[Bill Lorensen]

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