[Insight-developers] ITKv4: Features vs Clean up

Thu Jun 30 00:19:21 EDT 2011

Rupert,

Also we expect the refactored registration will be in gerrit by July 15...

Bill

On Wed, Jun 29, 2011 at 9:13 PM, brian avants <stnava at gmail.com> wrote:
> 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
>>
> _______________________________________________
> 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
>