[Insight-developers] Bug in Code/Review/itkOptImageToImageMetric.txx?

Hans Johnson hans-johnson at uiowa.edu
Thu Jun 11 16:37:39 EDT 2009


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/itkMattesMutualInformationI
mageToImageMetricTest.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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20090611/5f1bd4a4/attachment.htm>


More information about the Insight-developers mailing list