[Insight-developers] Warnings : Slicer & ITK

[Luis Ibanez]

Hi Steve,


Thanks for the compilation line. It seems that the
relevant flag is -Wall, and we have several gcc builds
with this flag on.

As you pointed out, the most likely cause is that this is
a case that is not being instantiated in the ITK testing suite.

I'll try to reproduce the warning in an ITK example.

It may actually be that two incompatible concepts
are being confounded in these filters...


   Thanks,


         Luis


------------------
On Mon, Apr 27, 2009 at 8:56 PM, Steve Pieper <pieper at bwh.harvard.edu> wrote:
> Hi Luis -
>
> Thanks for your help with this.
>
> Here's the code in question:
>
> http://viewvc.slicer.org/viewcvs.cgi/trunk/Applications/CLI/OtsuThresholdSegmentation.cxx?rev=9136&view=markup
>
> I believe it is actually an ITK example program that Bill modified a bit.
>  It looks to me like the types are actually correct - InternalImageType is
> unsigned long, which is the output of the ConnectedComponents filter.  So
> this doesn't fall into your 'risky' case.
>
> I guess this was a case that wasn't covered with the existing ITK tests.
>
> The compiler version, compile line and sample error messages are below.
>
> Best,
> Steve
>
>
> % g++ --version
> g++ (GCC) 4.1.2 20070925 (Red Hat 4.1.2-27)
>
> [ 93%] Building CXX object
> Applications/CLI/CMakeFiles/OtsuThresholdSegmentation.dir/OtsuThresholdSegmentation.cxx.o
> cd /workspace/pieper/slicer3/Slicer-3-4/Slicer3-build/Applications/CLI &&
> /usr/bin/g++    -ftemplate-depth-50 -Wall   -Wno-deprecated
> -ftemplate-depth-50  -ftemplate-depth-50 -Wall  -ftemplate-depth-50 -Wall -g

.....
>
> Luis Ibanez wrote:
>>
>> Hi Steve,
>>
>> Thanks for pointing this out.
>>
>> --
>>
>> Fixes for the warnings that you identified in ITK
>> have now been committed:
>>
>>
>> http://www.itk.org/cgi-bin/viewcvs.cgi/Code/BasicFilters/itkConnectedComponentImageFilter.h?root=Insight&r1=1.22&r2=1.23&sortby=date
>>
>> http://www.itk.org/cgi-bin/viewcvs.cgi/Code/BasicFilters/itkConnectedComponentImageFilter.txx?root=Insight&r1=1.31&r2=1.32&sortby=date
>>
>> http://www.itk.org/cgi-bin/viewcvs.cgi/Code/BasicFilters/itkRelabelComponentImageFilter.txx?root=Insight&r1=1.17&r2=1.18&sortby=date
>>
>> http://www.itk.org/cgi-bin/viewcvs.cgi/Code/BasicFilters/itkRelabelComponentImageFilter.h?root=Insight&r1=1.16&r2=1.17&sortby=date
>>
>>
>> The changes are a bit different of what is proposed in your patch.
>>
>> Instead of just adding the static_casts, we have introduced additional
>> typedefs to clarify the use of the two distinct concepts:
>>
>>     a) LabelType of a connected component.
>>     b) ObjectSizeType of a connected component.
>>
>>
>> I'm curious about why these warnings didn't show up in the
>> ITK Dashboard...
>>
>> One possibility may be that Slicer (or Slicer Modules) may be
>> instantiating connected component filters by using images of
>> pixel type different than "unsigned long".  Although it is
>> admissible to do so, it is also risky, specially if the pixel
>> type in question doesn't really has the capacity for counting
>> all the potential connected components that are present in
>> the image or that can be generated during the intermediate
>> procedure of labeling.
>>
>>
>> --
>>
>> Could you share with use the Warning flags that you are using ?
>> (and remind us of what platform pointed out these warnings)
>>
>> We will set them in one of the similar builds in the ITK Dashboards.
>>
>>
>> ---
>>
>>
>> About porting these changes back to ITK 3.10, we rather avoid this.
>>
>>
>> We only port back fixes for bugs, and only do it for the most recent
>> release branch. In this case ITK 3.12.
>>
>>
>> On the Bright side, ITK 3.14 will be out by the end of May.
>>
>>
>>   Please let us know if you find any other problems,
>>
>>
>>       Thanks
>>
>>
>>
>>            Luis
>>
>>
>>
>>
>> ---------------------
>> Steve Pieper wrote:
>>>
>>> Hi -
>>>
>>> Remember Bill's graffiti wisdom?
>>>
>>>
>>> http://www.na-mic.org/Wiki/index.php/2009_Winter_Project_Week_Compiler_Warnings:Slicer3_Graffiti
>>>
>>> Well, I took it to heart and cleaned up the remaining compile warnings in
>>> slicer3 (trunk and 3.4 release branch).  *Except* for a few lingering ones
>>> that trace back to ITK.
>>>
>>> The following changes are against ITK 3.12.  It would also be great if
>>> the same fixes (and the other recent fixes to this class) could go into ITK
>>> 3.10 since that's what we're using for slicer3.4.
>>>
>>> If we do this, then people who download and build the release branch
>>> would get 0 warnings and 0 errors, and we'd all like that, wouldn't we?
>>>
>>> Thanks,
>>> Steve
>>>
>>> p.s. please also doublecheck that the changes make sense - they work for
>>> my test cases and I believe the are correct.
>>>
>>>
>>> % cvs -q diff
>>> Index: Code/BasicFilters/itkConnectedComponentImageFilter.txx
>>> ===================================================================
>>> RCS file:
>>> /cvsroot/Insight/Insight/Code/BasicFilters/itkConnectedComponentImageFilter.txx,v
>>> retrieving revision 1.31
>>> diff -r1.31 itkConnectedComponentImageFilter.txx
>>> 600c600
>>> <   m_Consecutive[m_BackgroundValue] = m_BackgroundValue;
>>> ---
>>>  >   m_Consecutive[static_cast<unsigned long>(m_BackgroundValue)] =
>>> static_cast<unsigned long>(m_BackgroundValue);
>>> Index: Code/BasicFilters/itkRelabelComponentImageFilter.txx
>>> ===================================================================
>>> RCS file:
>>> /cvsroot/Insight/Insight/Code/BasicFilters/itkRelabelComponentImageFilter.txx,v
>>> retrieving revision 1.17
>>> diff -r1.17 itkRelabelComponentImageFilter.txx
>>> 105c105
>>> <       mapIt = sizeMap.find( inputValue );
>>> ---
>>>  >       mapIt = sizeMap.find( static_cast<unsigned long>(inputValue) );
>>> 109,110c109,110
>>> <         initialSize.m_ObjectNumber = inputValue;
>>> <         sizeMap.insert( MapValueType( inputValue, initialSize ) );
>>> ---
>>>  >         initialSize.m_ObjectNumber = static_cast<unsigned
>>> long>(inputValue);
>>>  >         sizeMap.insert( MapValueType( static_cast<unsigned
>>> long>(inputValue), initialSize ) );
>>> 213c213
>>> <       outputValue =
>>> static_cast<OutputPixelType>(relabelMap[inputValue]);
>>> ---
>>>  >       outputValue =
>>> static_cast<OutputPixelType>(relabelMap[static_cast<unsigned
>>> long>(inputValue)]);
>>> _______________________________________________
>>> NAMIC-Eng mailing list
>>> NAMIC-Eng at na-mic.org
>>> http://public.kitware.com/cgi-bin/mailman/listinfo/namic-eng
>>>
>