[Insight-developers] Removal of itkWarpJacobianDeterminantFilter.h
Johnson, Hans
hans-johnson at uiowa.edu
Fri Jul 11 08:59:28 EDT 2008
Tom,
Thanks for your comments. My understanding of a "Deformation Field
Jacobian" also makes the default behavior of the current filter meaningless.
It is also my opinion that the filter is not computing the correct result,
and that the comment in the header of that file is incorrect
< * should be 1.0. In general, in order to obtain the effective deformation
< * Jacobian determinant, 1.0 must be added to each Jacobian value in the
< * output.
---
> * is 1.0. In order to compute the effective deformation Jacobian determinant
> * 1.0 must be added to the diagonal elements of Jacobian prior to taking the
derivative.
> * i.e. det([ (1.0+dx/dx) dx/dy dx/dz ; dy/dx (1.0+dy/dy) dy/dz; dz/dx dz/dy
(1.0+dz/dz) ])
================================================
I did not want two slightly different versions of the same filter with
different template signatures. I also thought that it would be difficult
for a new user to determine which one to use because of the coding style and
template differences even though the logical differences were quite small.
I very much like the idea of deprecating the current filter (unless someone
can indicate that the answers produced are actually correct in some
circumstances). This would also free us to change the template parameter
signature of the filter without causing end user confusion about two filters
that on the surface appear to claim to do the same thing.
Whatever the final decision is, we need to highlight the differences between
the two filters and give appropriate use cases for each filter in the header
comments of both filters.
================================================
Proposal:
A) Revert DeformationFieldJacobianDeterminantFilter to it's previous state,
and mark it as deprecated.
B) Match the template parameters of WarpJacobianDeterminantFilter to that of
DeformationFieldJacobianDeterminantFilter so that the internal computations
of the jacobian can be done in either float or double. This also allows
easy replacement of the DeformationFieldJacobianDeterminantFilter through
search and replace.
C) Fast track the inclusion of itkWarpJacobianDeterminant filter out of
Review and into ITK. This should be possible because it is a bug fix, and
is the most reasonable way to preserve backwards compatibility. I've
submitted the bug already http://public.kitware.com/Bug/view.php?id=7327
I'll collect comments this weekend, and start working on this solution.
Thanks,
Hans
On 7/11/08 2:47 AM, "Tom Vercauteren" <tom.vercauteren at m4x.org> wrote:
> Hi Hans,
>
> Thanks for the heads up!
>
> I remember discussing the DeformationFieldJacobianDeterminantFilter
> issue some time ago. See the following posts:
> http://www.itk.org/pipermail/insight-users/2007-July/022858.html
> http://www.itk.org/pipermail/insight-users/2007-July/022866.html
>
> Adding a flag to DeformationFieldJacobianDeterminantFilter and
> removing WarpJacobianDeterminantFilter is OK for me. The only thing
> that bugs me is that the default behavior of
> DeformationFieldJacobianDeterminantFilter (i.e. the current one) would
> be to compute a somewhat "meaningless" quantity. And as far as I
> understand it, it might not be possible to set
> m_GenerateEffectiveJacobianDeterminant to true by default since this
> would break backwards compatibility.
>
> However, if we consider the current behavior of
> DeformationFieldJacobianDeterminantFilter to be a bug (which I do),
> changing the default behavior should IMHO not be considered as
> breaking backwards compatibility. Other people might have a different
> feeling about that though:
> http://www.vtk.org/Wiki/ITK_Backward_Compatibility_Open_Discussion
>
> Another option might be to;
> 1) Allow both behaviors in WarpJacobianDeterminantFilter
> 2) Set the default behavior of WarpJacobianDeterminantFilter to be
> m_GenerateEffectiveJacobianDeterminant=true
> 3) Put WarpJacobianDeterminantFilter in the main ITK tree (not in Review)
> 4) Deprecate DeformationFieldJacobianDeterminantFilter
>
> The advantage of this would also be that the name of the filter
> WarpJacobianDeterminantFilter would better represent the default
> behavior.
>
> Regards,
>
> Tom
>
> On Fri, Jul 11, 2008 at 3:29 AM, Johnson, Hans <hans-johnson at uiowa.edu> wrote:
>> Luis,
>>
>> I see that you are adding the Diffeomorphic Demons to ITK Review. GREAT!
>> I've been working with that code for a few months now, and it really is a
>> nice addition to ITK.
>>
>> I was reviewing the code in itkWarpJacobianDeterminantFilter.h, and
>> comparing it against itkDeformationFieldJacobianDeterminantFilter.h It
>> seems that all the functionality of the WarpJacobianDeterminant can be added
>> to the DeformationFieldJacobianDeterminant with little effort. I've
>> attached my local copies that have the necessary changes (it passes
>> regression test for being backwards compatible, and it address the comment
>> in itkDeformationFieldJacobianDeterminantFilter.
>>
>> NOTE 1: itkWarpJacobianDeterminant and
>> itkDeformationFieldJacobianDeterminant have different template
>> parameterization, but this seems to be a style choice.
>> NOTE 2: I've added another private member variable to precompute the
>> 0.5*weightings. This is not a necessary change for getting the new
>> functionality.
>>
>> I'd like to know your thoughts if this seems like a reasonable change. If
>> you agree, I will work through committing the enhanced version of
>> itkDeformationFieldJacobianDeterminant, and removing the
>> itkWarpJacobianDeterminantFilter from itk Review.
>>
>> Thanks,
>> Hans
>>
>> ===== Patch ===
>> Index: itkDeformationFieldJacobianDeterminantFilter.h
>> ===================================================================
>> RCS file:
>> /cvsroot/Insight/Insight/Code/BasicFilters/itkDeformationFieldJacobianDetermi
>> nantFilter.h,v
>> retrieving revision 1.3
>> diff -b -r1.3 itkDeformationFieldJacobianDeterminantFilter.h
>> 6c6
>> < Date: $Date: 2005-08-24 18:00:04 $
>> ---
>>> Date: $Date: 2005/08/24 18:00:04 $
>> 42,44c42,44
>> < * should be 1.0. In general, in order to obtain the effective deformation
>> < * Jacobian determinant, 1.0 must be added to each Jacobian value in the
>> < * output.
>> ---
>>> * is 1.0. In order to compute the effective deformation Jacobian
>>> determinant
>>> * 1.0 must be added to the diagonal elements of Jacobian prior to taking
>>> the derivative.
>>> * i.e. det([ (1.0+dx/dx) dx/dy dx/dz ; dy/dx (1.0+dy/dy) dy/dz; dz/dx
>>> dz/dy (1.0+dz/dz) ])
>> 240d239
>> < unsigned i, j;
>> 243c242
>> < for (i = 0; i < ImageDimension; ++i)
>> ---
>>> for (unsigned int i = 0; i < ImageDimension; ++i)
>> 245c244
>> < for (j = 0; j < VectorDimension; ++j)
>> ---
>>> for (unsigned int j = 0; j < VectorDimension; ++j)
>> 247,248c246
>> < J[i][j] = m_DerivativeWeights[i]
>> < * 0.5 * (it.GetNext(i)[j] - it.GetPrevious(i)[j]);
>> ---
>>> J[i][j] = m_HalfDerivativeWeights[i] * (it.GetNext(i)[j] -
>>> it.GetPrevious(i)[j]);
>> 249a248,251
>>> if(m_GenerateEffectiveJacobianDeterminant)
>>> {
>>> // add one on the diagonal to consider the warping and not only
>>> the deformation field
>>> J[i][i] += 1.0;
>> 251,252c253,255
>> <
>> < return vnl_det(J);
>> ---
>>> }
>>> const TRealType value=vnl_det(J);
>>> return value;
>> 256a260,261
>>> /** Pre-compute 0.5*m_DerivativeWeights since that is the only thing
>>> used in the computations. */
>>> TRealType
>>>
m_HalfDerivativeWeights[itk::GetImageDimension<TInputImage>::ImageDimension]>>>
;
>> 267a273
>>> bool m_GenerateEffectiveJacobianDeterminant;
>> Index: itkDeformationFieldJacobianDeterminantFilter.txx
>> ===================================================================
>> RCS file:
>> /cvsroot/Insight/Insight/Code/BasicFilters/itkDeformationFieldJacobianDetermi
>> nantFilter.txx,v
>> retrieving revision 1.3
>> diff -b -r1.3 itkDeformationFieldJacobianDeterminantFilter.txx
>> 6c6
>> < Date: $Date: 2006-01-11 19:43:31 $
>> ---
>>> Date: $Date: 2006/01/11 19:43:31 $
>> 37d36
>> < unsigned int i;
>> 40c39
>> < for (i = 0; i < ImageDimension; i++)
>> ---
>>> for (unsigned int i = 0; i < ImageDimension; i++)
>> 43a43
>>> m_HalfDerivativeWeights[i] = static_cast<TRealType>(0.5);
>> 44a45
>>> m_GenerateEffectiveJacobianDeterminant=false;//Needs to be false for
>>> backwards compatibility;
>> 58a60
>>> m_HalfDerivativeWeights[i] = 0.5*data[i];
>> 79a82
>>> m_HalfDerivativeWeights[i] = static_cast<TRealType>(0.5);
>> 156a160
>>> m_HalfDerivativeWeights[i]=0.5*m_DerivativeWeights[i];
>> 240a245,248
>>> os << indent << "m_HalfDerivativeWeights = ";
>>> for (i = 0; i < ImageDimension; i++)
>>> { os << m_HalfDerivativeWeights[i] << " "; }
>>> os << std::endl;
>> 244a253,254
>>> os << indent << "m_GenerateEffectiveJacobianDeterminant = " <<
>>> m_GenerateEffectiveJacobianDeterminant
>>> << std::endl;
>> _______________________________________________
>> Insight-developers mailing list
>> Insight-developers at itk.org
>> http://www.itk.org/mailman/listinfo/insight-developers
>>
>>
More information about the Insight-developers
mailing list