[Insight-developers] Bug in Code/Review/itkOptImageToImageMetric.txx?
Luis Ibanez
luis.ibanez at kitware.com
Thu Jun 11 21:48:43 EDT 2009
Hans,
Thanks a lot for tracking and fixing this bug.
Luis
-----------------------
Hans Johnson wrote:
> Committed fix to previously reported bug.
>
> /cvsroot/Insight/Insight/Code/Review/itkOptImageToImageMetric.h,v <--
> itkOptImageToImageMetric.h
> new revision: 1.23; previous revision: 1.22
> /cvsroot/Insight/Insight/Code/Review/itkOptImageToImageMetric.txx,v <--
> itkOptImageToImageMetric.txx
> new revision: 1.30; previous revision: 1.29
>
> --
> Hans J. Johnson, Ph.D.
> Associate Faculty
> hans-johnson at uiowa.edu
> 319 353 8587
>
>
>
> ------------------------------------------------------------------------
> *From: *Hans Johnson <hans-johnson at uiowa.edu>
> *Date: *Thu, 11 Jun 2009 14:15:14 -0500
> *To: *Hans Johnson <hans-johnson at uiowa.edu>, Luis Ibanez
> <luis.ibanez at kitware.com>, Kent Williams <norman-k-williams at uiowa.edu>
> *Cc: *ITK <insight-developers at itk.org>
> *Subject: *Re: [Insight-developers] Bug in
> Code/Review/itkOptImageToImageMetric.txx?
>
> Luis,
>
> I have committed a test that now exercises the situation described
> below. It should start failing on the dashboard for those builds that
> use ITK_USE_OPTIMIZED_REGISTRATION_METHODS:BOOL=ON.
>
> cvs commit -m"BUG:0009139: Added a test that fails because the number of
> samples is internally reduced when the fixed image mask is smaller than
> the number of samples requested. This leads to an inconsistent
> initialization of several variables. The solution listed in bug 0009139
> ensures that all internal arrays are re-initialized to the correct
> length if this internal reduction in samples occurs."
> Testing/Code/Algorithms/itkMattesMutualInformationImageToImageMetricTest.cxx
> Committer: Hans Johnson <hjohnson at mail.psychiatry.uiowa.edu>
> /cvsroot/Insight/Insight/Testing/Code/Algorithms/itkMattesMutualInformationImageToImageMetricTest.cxx,v
> <--
> Testing/Code/Algorithms/itkMattesMutualInformationImageToImageMetricTest.cxx
> new revision: 1.21; previous revision: 1.20
>
>
> ====================
> I’ve not yet committed the fix to the code that is attached to the bug
> report. I’ll do that in about 1 hour.
>
> Hans
> --
> Hans J. Johnson, Ph.D.
> Associate Faculty
> hans-johnson at uiowa.edu
> 319 353 8587
>
>
>
> ------------------------------------------------------------------------
> *From: *Hans Johnson <hans-johnson at uiowa.edu>
> *Date: *Thu, 11 Jun 2009 11:15:26 -0500
> *To: *Luis Ibanez <luis.ibanez at kitware.com>, Kent Williams
> <norman-k-williams at uiowa.edu>
> *Cc: *ITK <insight-developers at itk.org>
> *Subject: *Re: [Insight-developers] Bug in
> Code/Review/itkOptImageToImageMetric.txx?
>
> Luis,
>
> I think I’ve finally tracked down the exact problem.
>
> Much of the code in ::MultiThreadingInitialize has a dependency on the
> variable m_NumberOfFixedImageSamples for determining the length of the
> weights arrays. This is OK if that variable were only set from the
> class API before computing the registration. Unfortunately, after
> initialization it is possible that the FixedImageRegion, or the number
> of voxels available in the FixedImage mask can change this value after
> initialization to a smaller number. If that is the case, then the size
> of the weights arrays also need to be re-adjusted in size. Ultimately
> the segmentation fault arises in a for loop over several arrays that
> should all be the same length (i.e. The new smaller value of
> m_NumberOfFixedImageSamples), but the loop index is specified as one of
> the arrays that has not been adjusted to the smaller size.
>
> Currently the code changes I have pass all the old regression test, and
> produce a good result in the “real world” code I have (where it was
> previously segmentation faulting). I do not yet have a regression test
> that fails with the old code and succeeds with the new code.
>
> I’ve spent more time trying to make a small test case than it took to
> actually fix the bug that was discovered by running a real test case.
> Currently when attempting to make a test case that only causes this
> problem, other error checking is stepping in front of this and not even
> getting to cause this problem.
>
> I’ve added bug number “0009139” with a proposed patch that still needs
> testing.
>
> Hans
>
> --
> Hans J. Johnson, Ph.D.
> Associate Faculty
> hans-johnson at uiowa.edu
> 319 353 8587
>
>
>
> ------------------------------------------------------------------------
> *From: *Luis Ibanez <luis.ibanez at kitware.com>
> *Date: *Wed, 10 Jun 2009 12:57:56 -0400
> *To: *Kent Williams <norman-k-williams at uiowa.edu>
> *Cc: *ITK <insight-developers at itk.org>
> *Subject: *Re: [Insight-developers] Bug in
> Code/Review/itkOptImageToImageMetric.txx?
>
>
> Hi Kent,
>
> The number of fixed samples is updated according the number
> of pixels inside the (potential) fixed image mask in lines: 454-513,
>
> more specifically in 467-472
>
> if ( count > maxcount || randIter.IsAtEnd() )
> {
> m_NumberOfFixedImageSamples = samples_found;
> samples.resize(samples_found);
> break;
> }
>
>
> but... of course... it is always possible that there is a bug
> and we missed some combination of circumstances.
>
>
> Any chance that we could reduce this to a minimal case ?
>
>
> E.g. I would think in taking one of the examples in
> Insight/Examples/Registration and adding a Mask to it,
> so that we can reproduce this in a small test case.
>
> I'll be happy to help setting this up.
>
>
>
> BTW: In your case are us using AllPixels or are you
> letting the Metric subsample the image ?
>
> Depending on that, the code will take two different paths.
>
>
> Thanks
>
>
> Luis
>
>
> --------------------------------------------------------------------------------------------------
> On Wed, Jun 10, 2009 at 12:27 PM, kent williams
> <norman-k-williams at uiowa.edu> wrote:
>
> We ran into a problem that Hans could probably explain better than
> I: We are
> getting core dumps in itkOptImageToImageMetric.txx.
>
> The problem is that there is a member array
> m_BSplineTransformWeightsArray,
> that gets sized based on m_NumberOfFixedImageSamples and
> m_NumBSplineWeights. It's getting allocated (in our case to 10,000
> samples), but when it gets filled in (in
> PreComputeTransformValues()), every
> element is not set -- some are left with a value of zero or some random
> value.
>
> I think the problem is that the iteration over weights doesn't take into
> account the reduced actual sample count due to using masks.
>
>
> if(sampleOk)
> {
> // If the transform is BSplineDeformable, we can use the
> precomputed
> // weights and indices to obtained the mapped position
> const WeightsValueType * weights = if(sampleOk)
> {
> // If the transform is BSplineDeformable, we can use the
> precomputed
> // weights and indices to obtained the mapped position
> const WeightsValueType * weights =
>
> m_BSplineTransformWeightsArray[sampleNumber];
> const IndexValueType * indices =
>
> m_BSplineTransformIndicesArray[sampleNumber];
>
> for( unsigned int j = 0; j < FixedImageDimension; j++ )
> {
> mappedPoint[j] =
> m_BSplinePreTransformPointsArray[sampleNumber][j];
> }
>
> for ( unsigned int k = 0; k < m_NumBSplineWeights; k++ )
> {
> for ( unsigned int j = 0; j < FixedImageDimension; j++ )
> {
> //CRASHES HERE line 908
> mappedPoint[j] += weights[k] * m_Parameters[ indices[k]
> + m_BSplineParametersOffset[j] ];
> }
> }
> }
> }
>
>
> m_BSplineTransformWeightsArray[sampleNumber];
> const IndexValueType * indices =
>
> m_BSplineTransformIndicesArray[sampleNumber];
>
> for( unsigned int j = 0; j < FixedImageDimension; j++ )
> {
> mappedPoint[j] =
> m_BSplinePreTransformPointsArray[sampleNumber][j];
> }
>
> for ( unsigned int k = 0; k < m_NumBSplineWeights; k++ )
> {
> for ( unsigned int j = 0; j < FixedImageDimension; j++ )
> {
> mappedPoint[j] += weights[k] * m_Parameters[ indices[k]
> +
> m_BSplineParametersOffset[j] ];
> }
> }
> }
> }
>
>
>
>
>
>
> ------------------------------------------------------------------------
> _______________________________________________
> Powered by www.kitware.com
>
> Visit other Kitware open-source projects at
> http://www.kitware.com/opensource/opensource.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
>
> 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
>
More information about the Insight-developers
mailing list