[Insight-users] Re: some requested changes

[Luis Ibanez]

Hi Samson,

Thanks for your detailed message.

Here is what we added, following your suggestions:

1) itkGetConstReferenceMacro( Gradient, DerivativeType );

     in

     itkGradientDescentOptimizer.h
     itkRegularStepGradientDescentBaseOptimizer.h

     it wasn't added to the Conjugate gradient optimizer
     because this is a wrapper around a VNL optimizer.
     You may want to keep this as an open issue...


2)  You are right, Set/Get macros were missing in this file.
     The Set/Get methods are now using the macros.


3)  The min/max Fixed/Moving variables were added as you
     suggested.  I din't dare to remove the "evil" 0.001
     in this pass, so please keep this as an open issue
     we may have to pay a second visit to this file.


4)  Thanks for pointing this out.   You are right, the
     value to use is the number of paramters insteand of
     the image dimension.

     It seems that the reason why it works most of the time
     is that the trasnform used is the TranslationTransform
     that coincidentally has a number of parameters equal
     to the image dimension.   The changes were made in all
     the files except for

      itkMutualInformationHistogramTest.cxx

     which doesn't exist, maybe it is a typo in your email,
     could you please verify this and let us know.



Thanks a lot for this feedback,

These changes were commited to CVS, you may have
to update your checkout.

Please let us know if you find further problems,


Thanks



    Luis



-------------------
Samson Timoner wrote:

> 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
> 
> 
>