[Insight-developers] Develop ITK : Why dynamic_cast should never be used in Generic Programming.

[Luis Ibanez]

Nikos,


1) What ImageMetric are you using ?

2) Do you have enabled the flag:
    ITK_USE_OPTIMIZED_REGISTRATION_METHODS ?


If so,
you may be a victim of a mistake that was committed
in the ImageMetrics.

Inside the image metrics we are making the mistake
of using dynamic_cast in order to figure out when
the user is connecting a BSplineDeformable transform,
and in that case we treat that Transform differently.

Your renaming of the Transforms may be preventing
this dynamic_cast to succeed, and the code that has
been customized for BSplineDeformable transforms is
not being used in your case.

This exposes two errors in our current code:


   * Violation of encapsulation:

     The Metrics try to know too much about Transforms

   * Violation of Generic Programming agnostic

     The Metric should only rely on the API of
     Transforms, instead of expecting them to
     be of a specific type or hierarchy.


You will find the offending code at

       Insight/Code/Review/
          itkOptImageToImageMetric.txx


in lines 1371-1386:


   m_TransformIsBSpline = true;

   BSplineTransformType * testPtr2 = dynamic_cast<BSplineTransformType *>(
     this->m_Transform.GetPointer() );
   if( !testPtr2 )
     {
     m_TransformIsBSpline = false;
     m_BSplineTransform = NULL;
     itkDebugMacro( "Transform is not BSplineDeformable" );
     }
   else
     {
     m_BSplineTransform = testPtr2;
     m_NumBSplineWeights = m_BSplineTransform->GetNumberOfWeights();
     itkDebugMacro( "Transform is BSplineDeformable" );
     }



This code, and all the subsequent "if( transform is BSpline )"
are improper use of generic programming and should be refactored.

The problem that you are facing illustrates one of the reasons
why Generic Programming SHOULD NOT use "dynamic_cast" at all.



Please log a bug on this issue at

      http://public.kitware.com/Bug/my_view_page.php



In the short term,
you can get around this mistake in our code, by renaming
the type in line 419 of

      Insight/Code/Review/
          itkOptImageToImageMetric.h

Replace the declaration of that Transform with the one
that you are creating, and pay attention to its template
arguments. The type must be *identical*, (and that's
another of the handicaps introduced by dynamic_casting).



    Thanks


       Luis



-----------------------
Nikolaos Dikaios wrote:
> Hi Luis,
> 
> Yes. When i include the renamed files (i attached in my previous
> message) it becomes significantly slower compared to when i include the
> original itk ones.
> 
> Nikos
> 
> On Mon, 2009-07-06 at 12:41 -0400, Luis Ibanez wrote:
> 
>>Hi Nikos,
>>
>>
>>Are you saying that just renaming the classes
>>(without any other modification to their code)
>>results in a slowdown of the program  ?
>>
>>
>>
>>Please confirm,
>>
>>
>>   Thanks
>>
>>
>>      Luis
>>
>>
>>--------------------
>>Nikolaos Dikaios wrote:
>>
>>>Hi Luis, Daniel
>>>
>>>I have attached the files i included, they are identical to the itk ones
>>>i simply renamed them from itkBSpline... to ip_BSpline....
>>>
>>>At this stage i haven't passed my code so there shouldn't be any
>>>inefficiencies. This is the reason i cannot explain why it has become so
>>>slow compared to when i used the itk ones.
>>>
>>>I'll try to run one of the profilers you suggested.
>>>
>>>Thank you very much for your help,
>>>
>>>Nikos
>>>
>>>PS. This is the build mode i use 
>>>
>>>
>>>
>>>>>ccmake .
>>>
>>>
>>> CMAKE_BACKWARDS_COMPATIBILITY    2.4
>>> CMAKE_BUILD_TYPE                 Release
>>> CMAKE_INSTALL_PREFIX             /usr/local
>>> EXECUTABLE_OUTPUT_PATH
>>> ITK_DIR                          ~/itk3.8
>>> LIBRARY_OUTPUT_PATH
>>>
>>>then
>>>
>>>
>>>
>>>>>make
>>>
>>>
> 
>