[Insight-users] some requested changes

Samson Timoner samson at bwh . harvard . edu
Mon, 08 Dec 2003 19:46:16 -0500


I'd appreciate if the following changes could be made in CVS.
Only one of the changes is a bug:

1) I would like my observer to be observe the gradient vector
   in time during a minimization.

  In the following files (and any others you think appropriate):
    Numerics/itkGradientDescentOptimizer.h
    Numerics/itkConjugateGradientOptimizer.h
    Numerics/itkRegularStepGradientDescentBaseOptimizer.h

  Please add the following line:

  /** Get the current Derivative */
  itkGetConstMacro( Gradient, DerivativeType );

2) In itkHistogramImageToImageMetric.h

  First, this file is somewhat odd in that it does not
  use the itkGet/Set macros where it obviously should. Any idea why?

  However, I am sub-classing off this class and I need access to all
  the variables. I am missing one Get.  How about adding:

   /** Get the UsePaddingValue */
   itkGetConstReferenceMacro(UsePaddingValue, bool);

3) This comes from itkHistogramImageToImageMetric.txx. The bounds
   are the bounds that will be made for a histogram.
   You'll notice that an evil "0.001" that appears:

    // Initialize the upper and lower bounds of the histogram.
    m_LowerBound[0] = minFixed;
    m_LowerBound[1] = minMoving;
    m_UpperBound[0] = maxFixed + 0.001;
    m_UpperBound[1] = maxMoving + 0.001;

    The 0.001 exists so that the when you send the exact UpperBound
    to the histogram, a slight roundoff error does not make your
    number too big for the histogram. (Or, at least I think this
    is the reason.)

    However, while 0.001 may make sense if the numbers go from 0...1,
    it does not make sense if the numbers go from 1e-6 to 1e-7. 

   Please change to:

    m_LowerBound[0] = minFixed;
    m_LowerBound[1] = minMoving;
    m_UpperBound[0] = maxFixed  + (maxFixd -  minFixd)* 0.001;
    m_UpperBound[1] = maxMoving + (maxMoving - minMoving)* 0.001;

   As both Lower and UpperBound are doubles, this should be a bit
   better. 

   However, we should really try and get rid of the 0.001 as it will
   eventually not work for some application. Perhaps introducing a
   variable? Also, if we are worried about round off error on the high
   end, why aren't we worried about round off error on the low end?

4) In:
     itkMutualInformationHistogramTest.cxx
     itkHistogramImageToImageMetricTest.cxx
     itkMeanSquaresHistogramImageToImageMetricTest.cxx
     itkMutualInformationHistogramImageToImageMetricTest.cxx
     itkNormalizedMutualInformationHistogramImageToImageMetricTest.cxx

  All of these tests seg fault on my laptop. Why?
  In all these files, only some of the dimensions of the 
  the Derivative Step Length is set. So, in one of the files, we have:

      // Set scales for derivative calculation.
      ScalesType scales(transform->GetNumberOfParameters());
  
      for (unsigned int k = 0; k < ImageDimension; k++)
      scales[k] = 1;
  
      metric->SetDerivativeStepLengthScales(scales);

   It is pretty obvious that k should go from 0 to
   transform->GetNumberOfParameters, which is typically larger than
   ImageDimension. This error eventually caused a division by zero error,
   as the default value for the array is 0 and there is division by the
   DerivativeStepLengthScale. I do not know why other people did not
   find a division by zero error. I did

   All one needs to do is change the loop in all the files. Please change:

    >for (unsigned int k = 0; k < ImageDimension; k++)
    >  scales[k] = 1;

    to:

    >for (unsigned int k = 0; k < transform->GetNumberOfParameters(); k++)
    >  scales[k] = 1;

   While you are at it, in some of the files, the parameters are also
   set to 0 from k=0 to ImageDimension.


Thanks in advance for making these changes!

-- Samson