View Issue Details [ Jump to Notes ] | [ Print ] | ||||||||
ID | Project | Category | View Status | Date Submitted | Last Update | ||||
0008320 | ITK | public | 2008-12-18 22:15 | 2011-06-09 01:45 | |||||
Reporter | Hans Johnson | ||||||||
Assigned To | brian avants | ||||||||
Priority | high | Severity | major | Reproducibility | always | ||||
Status | closed | Resolution | fixed | ||||||
Platform | OS | OS Version | |||||||
Product Version | ITK-3-10 | ||||||||
Target Version | Fixed in Version | ||||||||
Summary | 0008320: Rigid3DTransform not setting state variables consistently | ||||||||
Description | The 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] !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! | ||||||||
Tags | No tags attached. | ||||||||
Resolution Date | |||||||||
Sprint | |||||||||
Sprint Status | completed | ||||||||
Attached Files | |||||||||
Relationships | |
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 [^] |
Notes |
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 |
Issue History |
Copyright © 2000 - 2018 MantisBT Team |