[Insight-developers] ITKImageFunction dependecy in Transform

[Matt McCormick]

Hi Alex,

Please review the following patch:

http://review.source.kitware.com/#change,2563

A patch that fixes the issue will follow sometime today on that branch.

Thanks,
Matt

On Wed, Aug 24, 2011 at 11:44 PM, Alexandre GOUAILLARD
<agouaillard at gmail.com> wrote:
> Dear all,
>
> The wrapping builds have been broken because of this issue for more
> than two weeks now.
>
> Could we move forward to a solution *soon* so that we can make
> progress on Wrapping? Almost no work could be done for the current
> Sprint, and there is only one sprint left before the beta.
>
> regards,
>
> alex.
>
>
> 2011/8/21 Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr>:
>>
>> Le 19 août 11 à 19:08, M Stauffer (V) a écrit :
>>
>>>>> This issue has popped up again, see below...
>>>>>
>>>>>>>>>> In rebasing the Transform changes patch set onto the latest
>>>>>>>>>
>>>>>>>>> itk master, I've
>>>>>>>>>>
>>>>>>>>>> got a dependency problem. DeformationFieldTransform
>>>>>>>>>> includes itkVectorLinearInterpolateImageFunction.h, which is
>>>>>>>>>> in ITKImageFunction. But adding this to
>>>>>>>>>
>>>>>>>>> Transform/itk-module.cmake causes a
>>>>>>>>>>
>>>>>>>>>> circular dependency. Any suggestions? I don't know how to
>>>>>>>>>
>>>>>>>>> handle this.
>>>>>>>>>
>>>>>>>>> That is a difficult problem.  Moving DeformationFieldTransform to
>>>>>>>>> ITKImageFunction would probably not be appropriate.  Is it
>>>>>>>
>>>>>>> possible to
>>>>>>>>>
>>>>>>>>> use a C++ forward declaration for
>>>>>>>>> VectorLinearInterpolateImageFunction?  I assume that any module
>>>>>>>>> depending on ITKTransform is also likely to depend on
>>>>>>>
>>>>>>> ITKImageFunction
>>>>>>>>>
>>>>>>>>> anyway.
>>>>>>>>
>>>>>>>> OK, a forward declare works. Of course I have to declare
>>>>>>
>>>>>> the template
>>>>>>>>
>>>>>>>> parrameters along with the forward declaration so it's a
>>>>>>>
>>>>>>> little messy in
>>>>>>>>
>>>>>>>> that it will need changing if the
>>>>>>>
>>>>>>> VectorLinearInterpolateImageFunction
>>>>>>>>
>>>>>>>> template parameters change. Although if the class template
>>>>>>>
>>>>>>> pararmeters
>>>>>>>>
>>>>>>>> change, lots of classes will change so I figure that's not a real
>>>>>>>> problem. So is forward-declaration like this ok in the ITK style?
>>>>>>>
>>>>>>> Great.
>>>>>>>
>>>>>>> ITK has very strong backwards compatibility, changes to the API like
>>>>>>> that will not take place after the ITKv4 release, and changes that
>>>>>>> break compatibility during ITKv4 need to have migration documents.
>>>>>>> http://www.vtk.org/Wiki/ITK_Release_4/Users_Migration_Guide
>>>>>>>
>>>>>>> Forward declaration is not typical ITK style.  However, if it is
>>>>>>> documented with a comment in the code explaining the need to break
>>>>>>> circular dependency, it could be OK.
>>>>>>>
>>>>>
>>>>> It turns out this has worked only because the cxx files where
>>>>> itkDisplacementTransform.h is included also include
>>>>> itkVectorInterpolateImageFunction.h and
>>>>> itkVectorLinearIntepolateImageFunction.h. Of course these have to be
>>>>> included somewhere for the forward-declare to work, but this
>>>>
>>>> seems bad
>>>>>
>>>>> for the use to have to include these other two headers separately.
>>>>>
>>>>> Looking at Core/ImageFunction, the only class that includes
>>>>
>>>> Transform is
>>>>>
>>>>> RayCastInterpolateImageFunction. Would it make sense to move this to
>>>>> another module so that ImageFunction needn't depend on Transform?
>>>>>
>>>>
>>>> Good detective work.  Maybe move RayCastInterpolateImageFunction to
>>>> Core/Common instead of the forward declare?
>>>>
>>>> Thanks,
>>>> Matt
>>>
>>> OK I've tried that and it works. I removed ITKTransform as a dependency
>>> of ITKImageFunction, and added ITKImageFunction as a dependency of
>>> ITKTransform. I moved RayCastInterpolateImageFunction to Core/Common.
>>>
>>
>> itkRayCastInterpolateImageFunction.h still includes itkTransform.h, right?
>>
>> If that's the case, it can't be moved to Core/Common, because itkTransform.h
>> is in ITKTransforms which is not a dependency of ITKCommon.
>>
>>> What's the protocol for going ahead with a move like this? Doesn't seem
>>> like much of a change since RayCastInterpolateImageFunction is still in
>>> Core. Matt, do you feel ok giving the final go-ahead for this, or do we
>>> get other approval too?
>>>
>>
>> Module groups don't have much meaning when dealing dependencies.
>>
>> This problem is preventing wrapping to build - the forward declaration is
>> not enough, the headers must be included - it would be fine to find a
>> solution.
>>
>> In that case I guess the only options are:
>>
>>  a. move RayCastInterpolateImageFunction to ITKTransforms
>>
>>  b. move DeformationFieldTransform to ITKImageFunction
>>
>>  c. move RayCastInterpolateImageFunction to another module which depends on
>> both ITKTransforms and ITKImageFunction. If no current module fit for that
>> task, then a new one should be created.
>>
>>  d. move DeformationFieldTransform to another module which depends on both
>> ITKTransforms and ITKImageFunction. If no current module fit for that task,
>> then a new one should be created.
>>
>> And of course adjust the module dependencies.
>>
>> I think my preference would go for c.
>>
>> Regards,
>>
>> Gaëtan
>>
>> --
>> Gaëtan Lehmann
>> Biologie du Développement et de la Reproduction
>> INRA de Jouy-en-Josas (France)
>> tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
>> http://mima2.jouy.inra.fr  http://www.itk.org
>> http://www.bepo.fr
>>
>>
>