View Issue Details Jump to Notes ] Print ]
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 
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.
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.
Resolution Date
Sprint
Sprint Statuscompleted
Attached Files

 Relationships

  Notes
(0014405)
Hans Johnson (developer)
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 (developer)
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 (manager)
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 (manager)
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 (developer)
2011-06-09 01:45

Fixed in git commit

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

 Issue History
Date Modified Username Field Change
2008-12-18 22:15 Hans Johnson New Issue
2008-12-18 23:52 Hans Johnson Note Added: 0014405
2008-12-19 10:55 Hans Johnson Note Added: 0014408
2008-12-26 13:10 Luis Ibanez Note Added: 0014443
2009-03-10 19:59 Hans Johnson Status new => assigned
2009-03-10 19:59 Hans Johnson Assigned To => Luis Ibanez
2010-11-07 01:29 Luis Ibanez Note Added: 0022974
2010-11-07 01:29 Luis Ibanez Assigned To Luis Ibanez => brian avants
2011-06-09 01:45 Cory W Quammen Sprint Status => completed
2011-06-09 01:45 Cory W Quammen Note Added: 0026791
2011-06-09 01:45 Cory W Quammen Status assigned => closed
2011-06-09 01:45 Cory W Quammen Resolution open => fixed


Copyright © 2000 - 2018 MantisBT Team