[ITK-dev] [ITK] Itk release branch demons registration
Bradly Lowekamp
blowekamp at mail.nih.gov
Tue Jan 19 13:52:37 EST 2016
Hello,
Sounds like you addressed a separate issue of “undefined behavior of signed integer overflow” [1], in additional to the “floating point division by zero” and “conversion truncation” I mentioned.
Amazing one line of computation can contain so many bugs :)
Brad
[1] http://stackoverflow.com/questions/16188263/is-signed-integer-overflow-still-undefined-behavior-in-c
> On Jan 19, 2016, at 1:20 PM, Hyun Jae Kang <hyunjae.kang at kitware.com> wrote:
>
> Hi Bradly,
>
> Thank you for your comments. And, I am sorry about lack of my previous explanation.
>
> I would like to share my observation about that part.
> I made debug messages to figure out why the run-time crash was happened on the previous code as the following:
>
> -------------------------------------------------------
> 2310: upperBound[i] ::2147483647
> 2310: lowerBound[i] ::-2147483648
> 2310: size[i] ::36
> 2310: MAX (signed int).::2147483647
>
> 1/1 Test #2310: itkSampleToHistogramFilterTest3 ...***Exception: Illegal 0.45 sec
> -------------------------------------------------------
>
> ----------------
> if ( size[i] > 0 )
> {
> - interval = static_cast<float>( upperBound[i] - lowerBound[i] )
> - / static_cast< MeasurementType >( size[i] );
> + interval = (static_cast<float>( upperBound[i] ) - static_cast<float>(lowerBound[i] ))
> + / static_cast< float >( size[i] );
> ---------------
>
> From the above debug messages and the previous code (see the above code snippet), the value of the numerator exceeded the max value of the assigned data type (signed int). And, the Clang compiler (3.0) generated a run-time crash on the case.
>
>
> If you have any question or comment, please let me know.
>
> Thanks,
>
> Hyun Jae
>
>
>
> On Tue, Jan 19, 2016 at 11:02 AM, Bradly Lowekamp <blowekamp at mail.nih.gov <mailto:blowekamp at mail.nih.gov>> wrote:
> Hello,
>
> Just to follow up.
>
> Your code seems to correct a long long time bug. That has had a work around [1] for at least one usage.
>
> The truncation of the “size” variable to the MeasurementType, can easily result in a divide by zero when the number of bins is NumericTraints<MeasurementType>::max() + 1.
>
> It may need to be noted in the release notes, that histogram calculations may change due to this correction, which can change the bounds of histograms.
>
> Good job tracking down the problem.
>
> Brad
>
>
> [1] https://github.com/InsightSoftwareConsortium/ITK/commit/4011c5d85e77387f59cadc0b12b79fdaa3251339 <https://github.com/InsightSoftwareConsortium/ITK/commit/4011c5d85e77387f59cadc0b12b79fdaa3251339>
>
>> On Jan 19, 2016, at 9:09 AM, Bradley Lowekamp <blowekamp at mail.nih.gov <mailto:blowekamp at mail.nih.gov>> wrote:
>>
>> Hello Hyun,
>>
>> From you description, I don’t understand what was the cause of the crass nor how this fixed the problem. Why was it the wrong type?
>>
>>
>> Brad
>>
>>
>>> On Jan 18, 2016, at 12:20 PM, Hyun Jae Kang <hyunjae.kang at kitware.com <mailto:hyunjae.kang at kitware.com>> wrote:
>>>
>>> Hi Bradley,
>>>
>>> Thank you for your comments on the code. The lines you mentioned in itkHistogram.hxx generated the run-time crash on Mac OSX 10.6 with clang compiler (3.0).
>>>
>>> When I found this error, I checked the data type of the local variable ( float interval ) and I put "float" in the lines to avoid the run-time error.
>>>
>>> This is why I chose "float" as the data type of data-casting.
>>>
>>> If I did something wrong, please let me know. I will fix it.
>>>
>>> Thanks,
>>>
>>> Hyun Jae
>>>
>>>
>>> On Mon, Jan 18, 2016 at 12:00 PM, <insight-developers-request at itk.org <mailto:insight-developers-request at itk.org>> wrote:
>>> Send Insight-developers mailing list submissions to
>>> insight-developers at itk.org <mailto:insight-developers at itk.org>
>>>
>>> To subscribe or unsubscribe via the World Wide Web, visit
>>> http://public.kitware.com/mailman/listinfo/insight-developers <http://public.kitware.com/mailman/listinfo/insight-developers>
>>> or, via email, send a message with subject or body 'help' to
>>> insight-developers-request at itk.org <mailto:insight-developers-request at itk.org>
>>>
>>> You can reach the person managing the list at
>>> insight-developers-owner at itk.org <mailto:insight-developers-owner at itk.org>
>>>
>>> When replying, please edit your Subject line so it is more specific
>>> than "Re: Contents of Insight-developers digest..."
>>>
>>>
>>> Today's Topics:
>>>
>>> 1. Re: [ITK] Itk release branch demons registration
>>> (Bradley Lowekamp)
>>>
>>>
>>> ----------------------------------------------------------------------
>>>
>>> Message: 1
>>> Date: Sun, 17 Jan 2016 18:13:44 -0500
>>> From: Bradley Lowekamp <brad at lowekamp.net <mailto:brad at lowekamp.net>>
>>> To: Bradley Lowekamp <brad at lowekamp.net <mailto:brad at lowekamp.net>>
>>> Cc: ITK <insight-developers at itk.org <mailto:insight-developers at itk.org>>
>>> Subject: Re: [ITK-dev] [ITK] Itk release branch demons registration
>>> Message-ID: <63AE857C-91FE-4B80-B31B-921A6CDAD2C6 at lowekamp.net <mailto:63AE857C-91FE-4B80-B31B-921A6CDAD2C6 at lowekamp.net>>
>>> Content-Type: text/plain; charset="utf-8"
>>>
>>> Just to follow up my git bisect narrowed the change down to this:
>>> commit 0fd462a3226e84bdce77b68830903b3888dab244
>>> Author: Hyun Jae Kang <hyunjae.kang at kitware.com <mailto:hyunjae.kang at kitware.com>>
>>> Date: Thu Dec 31 10:56:25 2015 -0500
>>>
>>> BUG: Fixed the runtime crash of ITKStatisticsTestDriver tests on OSX 10.6
>>>
>>> - Fixed the runtime crash of the following ITKStatisticsTestDriver's tests on
>>> OSX 10.6 ( clang 3.0 ) by modifying the itkHistogram's Initialize function
>>> to handle a data value out of range of a specific data type with
>>> type-casting operator.
>>>
>>> - itkSampleToHistogramFilterTest3 (ILLEGAL)
>>> - itkSampleToHistogramFilterTest7 (ILLEGAL)
>>>
>>> - These crash were reported at the following page:
>>> https://open.cdash.org/viewTest.php?onlyfailed&buildid=4170483 <https://open.cdash.org/viewTest.php?onlyfailed&buildid=4170483>
>>>
>>> Change-Id: I2063912edee79422d88125ffd5f7513c31a9e98c
>>>
>>> diff --git a/Modules/Numerics/Statistics/include/itkHistogram.hxx b/Modules/Numerics/Statistics/include/itkHistogram.hxx
>>> index 380b8d0..d11d788 100644
>>> --- a/Modules/Numerics/Statistics/include/itkHistogram.hxx
>>> +++ b/Modules/Numerics/Statistics/include/itkHistogram.hxx
>>> @@ -248,8 +248,8 @@ Histogram< TMeasurement, TFrequencyContainer >
>>> {
>>> if ( size[i] > 0 )
>>> {
>>> - interval = static_cast<float>( upperBound[i] - lowerBound[i] )
>>> - / static_cast< MeasurementType >( size[i] );
>>> + interval = (static_cast<float>( upperBound[i] ) - static_cast<float>(lowerBound[i] ))
>>> + / static_cast< float >( size[i] );
>>>
>>>
>>> Here is the failing example [1], which uses the HistogramMatching filter.
>>>
>>> I am not sure why float was choose at the type here. Are results being truncated now? When there was temporary double computation going on?
>>>
>>> It needs more investigation, as it?s changing the results of some filters? not sure how significant yet.
>>>
>>> Brad
>>>
>>>
>>>
>>>
>>> [1] https://github.com/SimpleITK/SimpleITK/blob/master/Examples/DemonsRegistration1.py#L41 <https://github.com/SimpleITK/SimpleITK/blob/master/Examples/DemonsRegistration1.py#L41>
>>> > On Jan 16, 2016, at 2:58 PM, Bradley Lowekamp <brad at lowekamp.net <mailto:brad at lowekamp.net>> wrote:
>>> >
>>> > Thanks. I'll run a git bisect tonight to try to figure it out.
>>> >
>>> >> On Jan 16, 2016, at 12:57 PM, Matt McCormick <matt.mccormick at kitware.com <mailto:matt.mccormick at kitware.com>> wrote:
>>> >>
>>> >> Hi,
>>> >>
>>> >> Nothing comes to mind.
>>> >>
>>> >> Here is the log for v4.9rc03..release:
>>> >>
>>> >> Bill Hoffman (3):
>>> >> COMP: fix 64 bit build warnings with windows auto-export on.
>>> >> COMP: work around for VS 2015 optimizer bug causing test failures.
>>> >> COMP: work around for VS 2015 optimizer bug causing test failures.
>>> >>
>>> >> Bradley Lowekamp (1):
>>> >> BUG: Fix precision with accumulation and scaling in AdaptiveHistogram
>>> >>
>>> >> Davis Vigneault (1):
>>> >> COMP: Include itkMacro.h in itkTestingMacros.h
>>> >>
>>> >> Hans Johnson (2):
>>> >> COMP: BSD command lines do not have --version
>>> >> COMP: Respect CMAKE_CXX_STANDARD during config
>>> >>
>>> >> Hyun Jae Kang (12):
>>> >> COMP: Fixed the compiler error of ITKCommon2TestDriver on OSX 10.6
>>> >> BUG: Fixed the runtime crash of vnl_test_complex on OSX 10.6
>>> >> BUG: Fixed the runtime crash of itkTimeProbeTest2
>>> >> BUG: Fixed the runtime crash of VideoSourceTest on OSX 10.6
>>> >> BUG: Fixed the runtime crash of ITKReviewTestDriver on OSX 10.6
>>> >> BUG: Fixed the runtime crash of ITKFastMarchingTestDriver's
>>> >> tests on OSX 10.6
>>> >> COMP: Fixed the data conversion warning messages of itkResourceProbe
>>> >> BUG: Fixed the runtime crash of ITKStatisticsTestDriver tests on OSX 10.6
>>> >> BUG: Fixed the runtime crash of itkBinaryShapeOpeningImageFilterTest1
>>> >> BUG: Fixed the runtime crash of test_pow_log on OSX 10.6
>>> >> BUG: Fixed the runtime crash of vnl_test_numeric_traits
>>> >> BUG: Exclude a test code of ITKLabelMapTestDriver on OSX 10.6
>>> >>
>>> >> Isaiah Norton (1):
>>> >> COMP: fix build with -std=c++11 when ITK/VXL initially configured without
>>> >>
>>> >> Matthew McCormick (6):
>>> >> BUG: Do not perform dynamic_cast in CompositeIOTransformIOHelper.
>>> >> COMP: Do not set property on itkhdf5 with ITK_USE_SYSTEM_HDF5.
>>> >> DOC: Update the location of the GNUPlot Software Guide scripts.
>>> >> DOC: Update the Software Guide repo location in Examples/README.txt.
>>> >> COMP: Add export specification for itk::ResourceProbe.
>>> >> COMP: Require NO_MODULE with DCMTK find_package.
>>> >>
>>> >> Michka Popoff (2):
>>> >> ENH: Allow Python 3 wrapping for Glue and Review Modules with VTK 7
>>> >> COMP: Re-enable review module for python 3 and older VTK's
>>> >>
>>> >> Sean McBride (1):
>>> >> DOC: Update some woefully out-of-date GDCM comments, and typos
>>> >>
>>> >> Seun Odutola (1):
>>> >> BUG: Improved itkAnalyzeImageIO to handle case insensitive extensions
>>> >>
>>> >> HTH,
>>> >> Matt
>>> >>
>>> >>> On Sat, Jan 16, 2016 at 12:18 PM, Bradley Lowekamp <brad at lowekamp.net <mailto:brad at lowekamp.net>> wrote:
>>> >>> I just updated SimpleITK to the release branch yesterday and there are some changes in the result of the demons registration. I'm not going to get a chance to look at it until Monday. The only change potentially related I know of is the rounding related numeric traits, but that does not seem likely to me.
>>> >>>
>>> >>> Any ideas why the Demons Registration would have changed fro rc3?
>>> >>>
>>> >>>> On Jan 16, 2016, at 12:13 PM, Bradley Lowekamp <brad at lowekamp.net <mailto:brad at lowekamp.net>> wrote:
>>> >>>>
>>> >>>> Hello,
>>> >>>>
>>> >>>> I updated SimpleITK
>>> >>>> _______________________________________________
>>> >>>> Powered by www.kitware.com <http://www.kitware.com/>
>>> >>>>
>>> >>>> Visit other Kitware open-source projects at
>>> >>>> http://www.kitware.com/opensource/opensource.html <http://www.kitware.com/opensource/opensource.html>
>>> >>>>
>>> >>>> Kitware offers ITK Training Courses, for more information visit:
>>> >>>> http://kitware.com/products/protraining.php <http://kitware.com/products/protraining.php>
>>> >>>>
>>> >>>> Please keep messages on-topic and check the ITK FAQ at:
>>> >>>> http://www.itk.org/Wiki/ITK_FAQ <http://www.itk.org/Wiki/ITK_FAQ>
>>> >>>>
>>> >>>> Follow this link to subscribe/unsubscribe:
>>> >>>> http://public.kitware.com/mailman/listinfo/insight-developers <http://public.kitware.com/mailman/listinfo/insight-developers>
>>> >>> _______________________________________________
>>> >>> Powered by www.kitware.com <http://www.kitware.com/>
>>> >>>
>>> >>> Visit other Kitware open-source projects at
>>> >>> http://www.kitware.com/opensource/opensource.html <http://www.kitware.com/opensource/opensource.html>
>>> >>>
>>> >>> Kitware offers ITK Training Courses, for more information visit:
>>> >>> http://kitware.com/products/protraining.php <http://kitware.com/products/protraining.php>
>>> >>>
>>> >>> Please keep messages on-topic and check the ITK FAQ at:
>>> >>> http://www.itk.org/Wiki/ITK_FAQ <http://www.itk.org/Wiki/ITK_FAQ>
>>> >>>
>>> >>> Follow this link to subscribe/unsubscribe:
>>> >>> http://public.kitware.com/mailman/listinfo/insight-developers <http://public.kitware.com/mailman/listinfo/insight-developers>
>>> > _______________________________________________
>>> > Powered by www.kitware.com <http://www.kitware.com/>
>>> >
>>> > Visit other Kitware open-source projects at
>>> > http://www.kitware.com/opensource/opensource.html <http://www.kitware.com/opensource/opensource.html>
>>> >
>>> > Kitware offers ITK Training Courses, for more information visit:
>>> > http://kitware.com/products/protraining.php <http://kitware.com/products/protraining.php>
>>> >
>>> > Please keep messages on-topic and check the ITK FAQ at:
>>> > http://www.itk.org/Wiki/ITK_FAQ <http://www.itk.org/Wiki/ITK_FAQ>
>>> >
>>> > Follow this link to subscribe/unsubscribe:
>>> > http://public.kitware.com/mailman/listinfo/insight-developers <http://public.kitware.com/mailman/listinfo/insight-developers>
>>> > _______________________________________________
>>> > Community mailing list
>>> > Community at itk.org <mailto:Community at itk.org>
>>> > http://public.kitware.com/mailman/listinfo/community <http://public.kitware.com/mailman/listinfo/community>
>>>
>>> -------------- next part --------------
>>> An HTML attachment was scrubbed...
>>> URL: <http://public.kitware.com/pipermail/insight-developers/attachments/20160117/188815c1/attachment-0001.html <http://public.kitware.com/pipermail/insight-developers/attachments/20160117/188815c1/attachment-0001.html>>
>>>
>>> ------------------------------
>>>
>>> Subject: Digest Footer
>>>
>>> _______________________________________________
>>> Insight-developers mailing list
>>> Insight-developers at itk.org <mailto:Insight-developers at itk.org>
>>> http://public.kitware.com/mailman/listinfo/insight-developers <http://public.kitware.com/mailman/listinfo/insight-developers>
>>>
>>>
>>> ------------------------------
>>>
>>> End of Insight-developers Digest, Vol 141, Issue 10
>>> ***************************************************
>>>
>>> _______________________________________________
>>> Powered by www.kitware.com <http://www.kitware.com/>
>>>
>>> Visit other Kitware open-source projects at
>>> http://www.kitware.com/opensource/opensource.html <http://www.kitware.com/opensource/opensource.html>
>>>
>>> Kitware offers ITK Training Courses, for more information visit:
>>> http://kitware.com/products/protraining.php <http://kitware.com/products/protraining.php>
>>>
>>> Please keep messages on-topic and check the ITK FAQ at:
>>> http://www.itk.org/Wiki/ITK_FAQ <http://www.itk.org/Wiki/ITK_FAQ>
>>>
>>> Follow this link to subscribe/unsubscribe:
>>> http://public.kitware.com/mailman/listinfo/insight-developers <http://public.kitware.com/mailman/listinfo/insight-developers>
>>
>> _______________________________________________
>> Powered by www.kitware.com <http://www.kitware.com/>
>>
>> Visit other Kitware open-source projects at
>> http://www.kitware.com/opensource/opensource.html <http://www.kitware.com/opensource/opensource.html>
>>
>> Kitware offers ITK Training Courses, for more information visit:
>> http://kitware.com/products/protraining.php <http://kitware.com/products/protraining.php>
>>
>> Please keep messages on-topic and check the ITK FAQ at:
>> http://www.itk.org/Wiki/ITK_FAQ <http://www.itk.org/Wiki/ITK_FAQ>
>>
>> Follow this link to subscribe/unsubscribe:
>> http://public.kitware.com/mailman/listinfo/insight-developers <http://public.kitware.com/mailman/listinfo/insight-developers>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://public.kitware.com/pipermail/insight-developers/attachments/20160119/41ff334f/attachment-0001.html>
More information about the Insight-developers
mailing list