[Insight-developers] Statistics library : Deprecation & New Framework

[Luis Ibanez]

Hi Stephen,

You are right, we had a sloppy approach with the image filters
due to the intertwined nature of the pipeline with the filters.

We were not strict enough on enforcing const-correctness in ITK
at the beginning, partly due to influence of VTK, that had managed
to live without this very useful C++ feature.

The unfortunate result is that image filters do not not enforce
const-correctness on the output at the compiler level, but still
*expect* that nobody (except the filter itself) would modify the
output images.

Today any user is (unfortunately) allowed to have a write access
to the output image of an image filter, just to find with surprise
and frustration that the next run of the pipeline destroys her/his
modifications in the image.

In other words we don't enforce a concept, instead, we simply pray
and hope that users will do the right thing, to not shoot themselves
on the foot.

I'll classify this as a bug, and propose that we fix it in the next
major number version of ITK (that will be 4.0).  Any user who may be
now accessing and changing the output of an ITK image filter is making
an incorrect use of the pipeline, and probably obtaining wrong results.

Of course in order to live with the burden of backward compatibility we
will have to fix this bug under the control of CMake variable that will
allow users to build ITK with the previous lack-const-correctness bug.



      Luis


-------------------------
Stephen R. Aylward wrote:
> Hi,
> 
> I understand your motivation.  Wasn't this a much debated topic during 
> ITK development - what was the final decision?   I ask because the 
> proposed statistics refactoring seems to not be consistent with the rest 
> of ITK.   In particular, image filters do not have const GetOutput 
> functions and do not return const images.
> 
> itkImageSource.h : line 113
>     OutputImageType * GetOutput(void);
> 
> with the following typedef in line 71
>     typedef TOutputImage OutputImageType
> 
> itkImageToImageFilter derives from ImageSource and does not change the 
> definition of GetOutput.
> 
> Again - I apologize if I am missing something in the code.
> 
> Thanks,
> Stephen
> 
> Luis Ibanez wrote:
> 
>>
>> Hi Stephen,
>>
>> Filters should return const objects because filters own their
>> output objects and are the only ones allowed to modify those
>> objects. The GetOutput() method itself should also be const,
>> because the invocation of that method doesn't change the state
>> of the filter.
>>
>> Given that we are already refactoring the Statistics Framework,
>> it is probably not worth to look at the API of the current
>> classes in Code/Numerics/Statistics.
>>
>> We anticipate that the entire directory will be deprecated in
>> a couple of released and the current code will just be carried
>> around under a backward-compatibility-enabler CMake variable.
>>
>>
>>
>> You may want to look a the new classes in:
>> http://www.itk.org/Wiki/Proposals:Refactoring_Statistics_Framework_2007
>>
>> svn checkout
>> http://www.na-mic.org/svn/NAMICSandBox/trunk/ITKStatisticsPipelineRefactoring 
>>
>>
>> They are conceptually described in:
>> http://www.itk.org/Wiki/Proposals:Refactoring_Statistics_Framework_2007_New_Statistics_Framework 
>>
>>
>>
>> These new classes has a *consistent const-correct API*:
>>
>> cd NAMICSandBox/trunk/ITKStatisticsPipelineRefactoring/Source
>>
>> $ grep "GetOutput()" *.h
>>
>> itkImageToListSampleFilter.h:  const ListSampleType * GetOutput() const;
>>
>> itkMeanFilter.h:  const MeasurementVectorDecoratedType * GetOutput() 
>> const;
>>
>> itkSampleClassifierFilter.h:  const OutputType * GetOutput() const;
>>
>> itkSampleToHistogramFilter.h:  const HistogramType  * GetOutput() const;
>>
>> itkSampleToSubsampleFilter.h:  const OutputType  * GetOutput() const;
>>
>> itkScalarImageToCooccurrenceListSampleFilter.h:  const SampleType *
>> GetOutput() const;
>>
>> itkScalarImageToCooccurrenceMatrixFilter.h:  const HistogramType * 
>> GetOutput() const;
>>
>>
>> ----
>>
>>
>> Following the Backward Compatibility policy of ITK:
>> http://www.itk.org/Wiki/Proposals:DeprecationProcedure
>> We have created a Migration Guide:
>> http://www.itk.org/Wiki/Proposals:Refactoring_Statistics_Framework_2007_Migration_Users_Guide 
>>
>> explaining users of the current framework what they should
>> do to migrate to the new framework.
>>
>>
>>
>>     Luis
>>
>>
>>
>>
>> ------------------------
>> Stephen R. Aylward wrote:
>>
>>> Hi,
>>>
>>> Am I missing some deeper motivation:  ImageToImageFilter (via 
>>> ImageSource) has a GetOutput that returns a non-const object; 
>>> however, the statistics library (code/numerics/statistics/) has some 
>>> classes whose GetOutput returns a const object 
>>> (itkImageToHistogramGenerator and 5 others) and others that return a 
>>> non-const object (itkKdTreeGenerator and 5 others).
>>>
>>> I recall this discussion only vaguely.
>>>
>>> Should the statistics library be updated to be internally consistent 
>>> and consistent with the image filters?   Seems like making all 
>>> non-const would be backward compatible...seems like Luis had an 
>>> argument against this at one time :)
>>>
>>> Thanks,
>>> Stephen
>>>
>>
>