[Insight-developers] Warnings : Slicer & ITK
Luis Ibanez
luis.ibanez at kitware.com
Tue Apr 28 09:56:39 EDT 2009
Hi Steve,
I'm not being able to reproduce the warning message that you reported.
I'm compiling the code from:
http://viewvc.slicer.org/viewcvs.cgi/trunk/Applications/CLI/OtsuThresholdSegmentation.cxx?rev=9136&view=markup
against a version of ITK that doesn't have last night commits.
I'm using the CXX_FLAGS: -Wall -Wshadow -Wcast-qual
Looking at the warnings in today's Slicer3 Dashboard,
http://www.cdash.org/CDash/viewBuildError.php?type=1&buildid=322001
the warnings that are related to the itkRelabelConnectedComponents
filter seem to be:
itkRelabelComponentImageFilter.txx:109: warning: converting to 'long
unsigned int' from 'float'
and are the result of attempting to instantiate this filter over an
input and output image of pixel type "float":
itk::RelabelComponentImageFilter<TInputImage, TOutputImage>::GenerateData()
[with TInputImage = itk::Image<float, 3u>, TOutputImage =
itk::Image<float, 3u>]':
and an input and output images of pixel type "double"
In member function 'void itk::RelabelComponentImageFilter<TInputImage,
TOutputImage>::GenerateData() [with TInputImage = itk::Image<double,
3u>, TOutputImage = itk::Image<double, 3u>]':
both of which can easily be considered inappropriate type for representing
the labels resulting from a segmentation.
The problem seem to originate from:
Slicer3/Libs/vtkITK/vtkITKIslandMath.cxx
which is instantiating images of type double and float in lines 170-171.
I would argue that the warnings are real and
that they are indicating a conceptual error in
the instantiation of the filter.
My suggestion will be to remove lines 170 and 171
from vtkITKIslandMath.cxx
Luis
-------------------------------------------------------------------------------
On Mon, Apr 27, 2009 at 10:14 PM, Luis Ibanez <luis.ibanez at kitware.com> wrote:
> 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
>>>>
>>
>
More information about the Insight-developers
mailing list