MantisBT - ITK
View Issue Details
IDProjectCategoryView StatusDate SubmittedLast Update
0008320ITKpublic2008-12-18 22:152011-06-09 01:45
ReporterHans Johnson 
Assigned Tobrian avants 
PriorityhighSeveritymajorReproducibilityalways
StatusclosedResolutionfixed 
PlatformOSOS Version
Product VersionITK-3-10 
Target VersionFixed in Version 
Resolution Date
Sprint
Sprint Statuscompleted
Summary0008320: Rigid3DTransform not setting state variables consistently
DescriptionThe two methods of setting the translation of a Rigid3DTransform do not produce a consistent internal state.


This is particularly problematic because when the SetTranslation() is used, it produces a rigid transformation that will work, but when the GetParameters() call is made imediatly after the SetTranslation call, it reports a zero translation in the parameters. The is catostrophic because writing transforms out to disk depends on the GetParameters calls.

The problem arises from inconsistent conversions between the m_Offset and the m_Translation internal variables.
Steps To Reproduce
Additional Information"!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" << std::endl;
    itk::Matrix<double, 3, 3> R=ZeroCenteredTransform->GetRotationMatrix();
    itk::Vector< double, 3> T;
    T[0]=100;T[1]=200;T[2]=300;

    itk::Rigid3DTransform<double>::Pointer
            r1=itk::Rigid3DTransform<double>::New();
    r1->SetRotationMatrix( R );
    r1->Translate( T );
    
    itk::Rigid3DTransform<double>::ParametersType p1;
    p1.set_size(12);
    p1=r1->GetParameters();
    std::cout << "r1\n" << r1 << std::endl;

    itk::Rigid3DTransform<double>::Pointer
           r2=itk::Rigid3DTransform<double>::New();
    itk::Rigid3DTransform<double>::ParametersType p2;
    p2.set_size(12);
    for(int r=0;r<3;r++)
      {
      for(int c=0;c<3;c++)
        {
        p2[r*3+c]=R[r][c];
        }
      }
    p2[ 9]=T[0];
    p2[10]=T[1];
    p2[11]=T[2];
    r2->SetParameters( p2 );

    std::cout << "r2\n" << r2 << std::endl;
    itk::Rigid3DTransform<double>::Pointer
         r3=itk::Rigid3DTransform<double>::New();
    r3->SetFixedParameters( r1->GetFixedParameters() );
    r3->SetParameters( r1->GetParameters() );
    std::cout << "r3\n" << r3 << std::endl;

    itk::Rigid3DTransform<double>::ParametersType p3;
    p3.set_size(12);
    p3=r3->GetParameters();

    std::cout << p1 << "\n" << p2 << "\n" << p3 << std::endl;

    std::cout << "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" << std::endl;


!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
r1
Rigid3DTransform (0x56c670)
  RTTI typeinfo: itk::Rigid3DTransform<double>
  Reference Count: 2
  Modified Time: 32843
  Debug: Off
  Observers:
    none
  Matrix:
    0.99761 0.05864 0.036553
    -0.0657556 0.968201 0.241377
    -0.0212363 -0.243204 0.969743
  Offset: [100, 200, 300]
  Center: [0, 0, 0]
  Translation: [0, 0, 0]
  Inverse:
    0.99761 -0.0657556 -0.0212363
    0.05864 0.968201 -0.243204
    0.036553 0.241377 0.969743
  Singular: 0

r2
Rigid3DTransform (0x56c940)
  RTTI typeinfo: itk::Rigid3DTransform<double>
  Reference Count: 2
  Modified Time: 32847
  Debug: Off
  Observers:
    none
  Matrix:
    0.99761 0.05864 0.036553
    -0.0657556 0.968201 0.241377
    -0.0212363 -0.243204 0.969743
  Offset: [100, 200, 300]
  Center: [0, 0, 0]
  Translation: [100, 200, 300]
  Inverse:
    0.99761 -0.0657556 -0.0212363
    0.05864 0.968201 -0.243204
    0.036553 0.241377 0.969743
  Singular: 0

r3
Rigid3DTransform (0x56ccd0)
  RTTI typeinfo: itk::Rigid3DTransform<double>
  Reference Count: 2
  Modified Time: 32852
  Debug: Off
  Observers:
    none
  Matrix:
    0.99761 0.05864 0.036553
    -0.0657556 0.968201 0.241377
    -0.0212363 -0.243204 0.969743
  Offset: [0, 0, 0]
  Center: [0, 0, 0]
  Translation: [0, 0, 0]
  Inverse:
    0.99761 -0.0657556 -0.0212363
    0.05864 0.968201 -0.243204
    0.036553 0.241377 0.969743
  Singular: 0

[0.99761, 0.05864, 0.036553, -0.0657556, 0.968201, 0.241377, -0.0212363, -0.243204, 0.969743, 0, 0, 0]
[0.99761, 0.05864, 0.036553, -0.0657556, 0.968201, 0.241377, -0.0212363, -0.243204, 0.969743, 100, 200, 300]
[0.99761, 0.05864, 0.036553, -0.0657556, 0.968201, 0.241377, -0.0212363, -0.243204, 0.969743, 0, 0, 0]
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
TagsNo tags attached.
Relationships
Attached Files
Issue History
Date ModifiedUsernameFieldChange
2008-12-18 22:15Hans JohnsonNew Issue
2008-12-18 23:52Hans JohnsonNote Added: 0014405
2008-12-19 10:55Hans JohnsonNote Added: 0014408
2008-12-26 13:10Luis IbanezNote Added: 0014443
2009-03-10 19:59Hans JohnsonStatusnew => assigned
2009-03-10 19:59Hans JohnsonAssigned To => Luis Ibanez
2010-11-07 01:29Luis IbanezNote Added: 0022974
2010-11-07 01:29Luis IbanezAssigned ToLuis Ibanez => brian avants
2011-06-09 01:45Cory W QuammenSprint Status => completed
2011-06-09 01:45Cory W QuammenNote Added: 0026791
2011-06-09 01:45Cory W QuammenStatusassigned => closed
2011-06-09 01:45Cory W QuammenResolutionopen => fixed

Notes
(0014405)
Hans Johnson   
2008-12-18 23:52   
Further testing still needed, but I believe the correct way to compose with the translate is to add to the current translation, and then re-compute the offset values.



cvs diff itkRigid3DTransform.txx
Index: itkRigid3DTransform.txx
===================================================================
RCS file: /cvsroot/Insight/Insight/Code/Common/itkRigid3DTransform.txx,v
retrieving revision 1.34
diff -r1.34 itkRigid3DTransform.txx
158c158
< OutputVectorType newOffset = this->GetOffset();
---
> OutputVectorType newOffset = this->GetTranslation();
160c160
< this->SetVarOffset(newOffset);
---
> this->SetTranslation(newOffset);
(0014408)
Hans Johnson   
2008-12-19 10:55   
This patch passes all regression tests.


Index: Code/Common/itkRigid2DTransform.txx
===================================================================
RCS file: /cvsroot/Insight/Insight/Code/Common/itkRigid2DTransform.txx,v
retrieving revision 1.23
diff -r1.23 itkRigid2DTransform.txx
128c128
< OutputVectorType newOffset = this->GetOffset();
---
> OutputVectorType newOffset = this->GetTranslation();
130c130
< this->SetOffset(newOffset);
---
> this->SetTranslation(newOffset);
Index: Code/Common/itkRigid3DTransform.txx
===================================================================
RCS file: /cvsroot/Insight/Insight/Code/Common/itkRigid3DTransform.txx,v
retrieving revision 1.34
diff -r1.34 itkRigid3DTransform.txx
158c158
< OutputVectorType newOffset = this->GetOffset();
---
> OutputVectorType newOffset = this->GetTranslation();
160c160
< this->SetVarOffset(newOffset);
---
> this->SetTranslation(newOffset);
Index: Testing/Code/Common/itkRigid3DTransformTest.cxx
===================================================================
RCS file: /cvsroot/Insight/Insight/Testing/Code/Common/itkRigid3DTransformTest.cxx,v
retrieving revision 1.19
diff -r1.19 itkRigid3DTransformTest.cxx
48a49,106
> bool TestSettingTranslation(void)
> {
>
> itk::Matrix<double, 3, 3> R;
> R.SetIdentity();
> const double alpha = vnl_math::pi / 180.0;
> R[0][0] = cos( alpha );
> R[0][1] = sin( alpha );
> R[1][0] = -1.0 * sin( alpha );
> R[1][1] = cos( alpha );
>
>
> itk::Vector< double, 3> T; T[0]=100;T[1]=200;T[2]=300;
> itk::Rigid3DTransform<double>::Pointer r1=itk::Rigid3DTransform<double>::New();
> //r1->SetIdentity();
> r1->SetRotationMatrix( R );
> r1->Translate( T );
> itk::Rigid3DTransform<double>::ParametersType p1;
> p1.set_size(12);
> p1=r1->GetParameters();
>
> itk::Rigid3DTransform<double>::Pointer r2=itk::Rigid3DTransform<double>::New();
> itk::Rigid3DTransform<double>::ParametersType p2;
> p2.set_size(12);
> for(int r=0;r<3;r++)
> {
> for(int c=0;c<3;c++)
> {
> p2[r*3+c]=R[r][c];
> }
> }
> p2[ 9]=T[0];
> p2[10]=T[1];
> p2[11]=T[2];
> r2->SetParameters( p2 );
> itk::Rigid3DTransform<double>::Pointer r3=itk::Rigid3DTransform<double>::New();
> r3->SetFixedParameters( r1->GetFixedParameters() );
> r3->SetParameters( r1->GetParameters() );
>
> itk::Rigid3DTransform<double>::ParametersType p3;
> p3.set_size(12);
> p3=r3->GetParameters();
> if( (p1 == p2) && (p1 == p3))
> {
> return true;
> }
> else
> {
> std::cout << "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" << std::endl;
> std::cout << "r1\n" << r1 << std::endl;
> std::cout << "r2\n" << r2 << std::endl;
> std::cout << "r3\n" << r3 << std::endl;
> std::cout << p1 << "\n" << p2 << "\n" << p3 << std::endl;
> std::cout << "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!" << std::endl;
> }
>
> return false;
> }
651a710,715
> bool TranslationSettingOK=TestSettingTranslation();
> if( !TranslationSettingOK )
> {
> std::cerr << "Error: SetTranslation() did not result in consisent internal state for Rigid3DTransform." << std::endl;
> return EXIT_FAILURE;
> }
(0014443)
Luis Ibanez   
2008-12-26 13:10   
The itkRigid3DTransform class, after the refactoring of Matrix&Offset classes was meant to become an abstract class. As such, its GetParameters and SetParamter methods should never have to be invoked. Instead, the SetParameters() and GetParameters() method of its derived classes should be the ones called.

In principle this bug could be addressed by removing the itkNewMacro() from this class.

The class itself doesn't provide a parametrization. This is what the derived classes do. For example:

  Euler angles
  Quaternions
  Versors

It doesn't hurt to have a valid implementation of Get/SetParamters at the level of the Rigid3DTransform. However, this class simply shouldn't be used as a full fledged transform. (e.g. for example as part of an image registration framework).
(0022974)
Luis Ibanez   
2010-11-07 01:29   
A suggested patch has been pushed to Gerrit:
http://review.source.kitware.com/#change,314 [^]

It makes the itk::Rigid3DTransform to be an Abstract class.

Its itkNewMacro() has been removed.

This should be addressed as part of the image registration refactoring in ITKv4
(0026791)
Cory W Quammen   
2011-06-09 01:45   
Fixed in git commit

http://www.itk.org/gitweb?p=ITK.git;a=commit;h=788f89c390d1aac58a17cc6598bff2645e6149fd [^]