[Insight-developers] ITKv4: Features vs Clean up

Wed Jun 29 21:13:42 EDT 2011

rupert

help is very welcome.

refactored registration framework is here:

https://github.com/picslITK/ITK4_topics/tree/RegistrationRefactoringNew

take a look at :

ITK4_topics/Modules/Registration/Refactoring/test/itkDemonsImageToImageObjectRegistrationTest.cxx

which gives an example of a demons registration run in the standard
framework.  the framework is changing, though, so realize what's there
now is a mid-stream snapshot.

brian

On Wed, Jun 29, 2011 at 8:59 PM, Rupert Brooks <rupert.brooks at gmail.com> wrote:
> Hi Luis and everyone,
>
> As a pretty heavy user of ITK, I'm interested in helping out as well.
> At the moment im working on some registration problems and im trying
> to explore the refactored registration framework in ITK 4.
> As im going through the current master, im noticing various things
> ranging from typos to classes that seem to have gotten left behind to
> questionable math.  I'd be happy to give this information as feedback
> or fixes if its helpful.
>
> So question 1 - is the registration refactoring well represented in
> ITK master, or is there more work elsewhere?  Is this the right time
> to review it from a user perspective or is it premature?
>
> Question 2 - whats the best way to proceed?  Do i need to configure
> Gerrit access, etc as a developer to contribute to the textual notes
> you mentioned?
>
> Cheers,
> Rupert
> --------------------------------------------------------------
> Rupert Brooks
> rupert.brooks at gmail.com
>
>
>
>
> On Fri, Jun 3, 2011 at 09:46, Luis Ibanez <luis.ibanez at kitware.com> wrote:
>> Hi Tom,
>>
>> Thanks for volunteering.
>>
>>
>> The plans for the global clean up are here:
>> http://www.itk.org/Wiki/ITK_Release_4/Global_Code_Review
>>
>> The procedure is described here
>> http://www.itk.org/Wiki/ITK_Release_4/Modularization/Code_Reviews/Process#Git-Based
>>
>> Essentially:
>>
>> We have a duplicate Git repository
>> https://github.com/InsightSoftwareConsortium/itk-retroactive-review
>>
>> that contains a .txt file for every source code file in ITK.
>>
>> Instead of making bug entries in MANTIS,
>> just edit those .txt files, and commit them.
>>
>> Of course, if some issues are actual bugs,
>> please feel free to report them in MANTIS.
>>
>>
>> More comments below...
>>
>>
>> On Fri, Jun 3, 2011 at 5:11 AM, Tom Vercauteren <tom.vercauteren at m4x.org> wrote:
>>> 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?
>>>
>>
>> http://www.itk.org/Wiki/ITK_Release_4/Modularization/Code_Reviews/Process#Git-Based
>> https://github.com/InsightSoftwareConsortium/itk-retroactive-review
>>
>>> 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.
>>>
>>
>> Fair enough...
>>
>>>
>>> 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
>>>
>>
>> Yes,
>> Cory, Arnaud, Gaetan and Brad L. have been improving
>> the support for multiple components...
>> We need raise the priority of this one,
>> since it is so fundamental for
>> Microscopy and for Remote Sensing.
>>
>>
>>>
>>> 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).
>>>
>>
>> Yeap, the API is inconsistent.
>> Plus some take Sigma, while others take Variance...
>>
>>>
>>> 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,
>>
>>
>> It certainly does.
>>
>>
>>   Many Thanks
>>
>>
>>        Luis
>>
>>
>>
>> -----------------------------
>>> 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
>>>>
>>>
>> _______________________________________________
>> 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
>>
> _______________________________________________
> 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
>