[Insight-developers] ITKv4: Features vs Clean up

[Tom Vercauteren]

Hi Luis,

I would like to help in the cleanup process (especially for the code I
worked on in the past) but have unfortunately very little time for it.
The least I could do is pinpoint some places that I believe require
cleanup. The problem is I don't really know if such a task is wanted
and if so, how it should be done. Should I file bug reports  for each
cleanup task and mark them as feature requests? But then, who would be
in charge of triaging these feature requests? Are other more effective
options available?

For the time being, there are a least a few places on the top of my
head that require cleanup (some of which might be considered as
features depending on how you look at it). Sorted by order of
importance:

1) Bad design in ImportImageContainer.
https://github.com/Kitware/ITK/blob/master/Modules/Core/Common/include/itkImportImageContainer.h
The memory allocation and deallocation functions have an inconsistent
signature. One uses a member and the others returns a pointer:
  virtual TElement * AllocateElements(ElementIdentifier size) const;
  virtual void DeallocateManagedMemory();

This makes it really difficult to override the class in a correct
manner. I have actually stumped into a bug in TestImportImageContainer
coming from this design issue ( that I haven't filed yet :( )
https://github.com/Kitware/ITK/blob/master/Modules/Core/Common/test/itkFactoryTestLib.cxx
With this TestImportImageContainer class, reallocating an image will
fail. This is bad since it will most certainly be the first thing
people will look at when trying to override the default memory
handling of ITK.


2) The unicode filename problem on windows.
Most of the low-level code necessary to deal with unicode filenames
through utf-8 encoding is already in the repository but it isn't used
yet. What still need to be done is use this low-level code for all IO
operations. More detail in:
http://www.itk.org/Bug/view.php?id=9623


3) There are a lot of useless vector-specific classes that were
introduced probably for MSVC 6. It might be the right time to merge
them back into the corresponding "scalar" classes.
http://itk.org/Bug/view.php?id=2712
http://www.itk.org/Wiki/ITK_Release_4/Wish_List#Image_Registration


4) Some default options are not the most meaningful ones:
http://www.itk.org/Wiki/ITK_Release_4/Wish_List#Backward_compatibility_and_cleanup
For example Gaussian smoothing uses either a sigma defined in terms of
physical space or pixel space depending on its implementation
(discrete versus recursive).


5) Others inconsistencies
http://www.itk.org/Bug/view.php?id=7351
http://www.itk.org/Bug/view.php?id=8944
http://www.itk.org/Wiki/ITK_Release_4/Wish_List#Make_the_boundary_conditions_usage_consistent_across_the_toolkit
For example the behavior of the interpolators outside the image domain
is not practical. This is one of the reason why I implemented
VectorLinearInterpolateNearestNeighborExtrapolateImageFunction
https://github.com/Kitware/ITK/blob/master/Modules/Nonunit/Review/include/itkVectorLinearInterpolateNearestNeighborExtrapolateImageFunction.h
Ideally, the functionnality of this class should be handled by
LinearInterpolateImageFunction.


Hope this helps,
Tom

On Thu, Jun 2, 2011 at 15:06, Luis Ibanez <luis.ibanez at kitware.com> wrote:
> I want to second Brad L. comment regarding
> our current misplaced focus on ITKv4.
>
> There is a disproportionate number of changes
> that are introducing new features in the toolkit,
> when we are supposed to be focused on cleaning
> up and restructuring for the next 10 years.
>
> I suggest that we prioritize every patch in Gerrit
> based on whether it is:
>
>
> 1) Bug fix
> 2) Clean up resulting from code reviews
> 3) Needed for refactoring (FEM, Regist, LevelSets)
> ....
> 99) New features
>
>
> We have limited time and resources to do all
> the refactoring and revision of ITKv4 and we
> are being side-tracked dealing with changes
> that are not part of the ITKv4 charter.
>
>
>       Luis
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.html
>
> Kitware offers ITK Training Courses, for more information visit:
> http://kitware.com/products/protraining.html
>
> Please keep messages on-topic and check the ITK FAQ at:
> http://www.itk.org/Wiki/ITK_FAQ
>
> Follow this link to subscribe/unsubscribe:
> http://www.itk.org/mailman/listinfo/insight-developers
>