Does an efficient cloning of multiple different classes with an override of InternalClone() exist?

clone

(Gordian) #1

Hello,

i am looking for a way to clone object with all parameters.I want to use this on multiple class objects like metrics, optimizer, interpolator etc. and I dont want to write new derived classes just for the cloning feature.
I am looking for an efficient way to clone a class with all parameters (which can be very different).

The two methods CreateAnother() and Clone() return in most cases an object without any parameters. In some cases the InternalClone() method (that is called inside Clone()) is overriden to copy the parameter.
See Transform.

Is there a general way to achieve this (by using marcos?). I cannot figure out how it is done efficiently (I guess otherwise it would been implemented).

My best guess was to create three macros. One for the first part of the InternalClone()

#define itkInternalCloneHeader(x)												\
 virtual typename itk::LightObject::Pointer InternalClone() const ITK_OVERRIDE		\
 {																				\
	typename itk::LightObject::Pointer loPtr = Superclass::InternalClone();			\
	std::cout << "\nCloning...\n" << std::endl;										\
	typename Self::Pointer rval = dynamic_cast<Self *>(loPtr.GetPointer());		\
	if (rval.IsNull())															\
	{																			\
		itkExceptionMacro(<< "downcast to type " << this->GetNameOfClass() << " failed.");\
	}

A second one for the copying of the parameter:

#define itkSetGetMacro(x) \
	rval->Set##x(this->Get##x());

And a last one for the footer:

#define itkInternalCloneFooter(x) \
	return loPtr;				  \
  }

All together can be written as and will copy one parameter x:

    itkInternalCloneHeader(0);
    itkSetGetMacro(x);
    itkInternalCloneFooter(0)

For every parameter another itkSetGetMacro(x) is needed. This does not save any work. I could not find a way to provide multiple arguments to this macro.
The idea of a loop inside the macro wont work as far my investigation went.

Are there any ideas we can discuss or is there a more elegant way for object cloning?

Thanks in advance,
Gordian


(Gordian) #2

Hi,

now i made a try to get a clone of the itk::AmoebaOptimizer.

To get a clone with the correct parameters I added in following classes following code.
In itkOptimizer.h, itkSingleValuedNonLinearVnlOptimizer.h, itkAmoebaOptimizer.h I added in the protected space:

/**
* Clone the current optimizer.
* This does a complete copy of the optimizer state to the new optimizer
*/
typename LightObject::Pointer InternalClone() const override;

In the body files is added:

itkOptimizer.cxx

LightObject::Pointer
Optimizer
::InternalClone() const
{
	// Default implementation just copies the parameters from
	// this to new optimizer.
	LightObject::Pointer loPtr = Superclass::InternalClone();
	Self::Pointer rval = dynamic_cast<Self *>(loPtr.GetPointer());
	if (rval.IsNull())
	{
		itkExceptionMacro(<< "downcast to type " << this->GetNameOfClass() << " failed.");
	}

	rval->SetScales(this->GetScales());
	rval->SetInitialPosition(this->GetInitialPosition());
	return loPtr;
}

itkSingleValuedNonLinearVnlOptimizer.cxx

LightObject::Pointer
	SingleValuedNonLinearVnlOptimizer
	::InternalClone() const
	{
		// Default implementation just copies the parameters from
		// this to new optimizer.
		LightObject::Pointer loPtr = Superclass::InternalClone();

		Self::Pointer rval = dynamic_cast<Self *>(loPtr.GetPointer());
		if (rval.IsNull())
		{
			itkExceptionMacro(<< "downcast to type " << this->GetNameOfClass() << " failed.");
		}
		rval->SetMaximize(this->GetMaximize());
		return loPtr;
	}

itkAmoebaOptimizer.cxx

LightObject::Pointer
AmoebaOptimizer
::InternalClone() const
{
	// Default implementation just copies the parameters from
	// this to new optimizer.
	LightObject::Pointer loPtr = Superclass::InternalClone();

	Self::Pointer rval = dynamic_cast<Self *>(loPtr.GetPointer());
	if (rval.IsNull())
	{
		itkExceptionMacro(<< "downcast to type " << this->GetNameOfClass() << " failed.");
	}
	
	rval->SetMaximumNumberOfIterations(this->GetMaximumNumberOfIterations());
	rval->SetOptimizeWithRestarts(this->GetOptimizeWithRestarts());
	rval->SetParametersConvergenceTolerance(this->GetParametersConvergenceTolerance());
	rval->SetFunctionConvergenceTolerance(this->GetFunctionConvergenceTolerance());
	rval->SetInitialSimplexDelta(this->GetInitialSimplexDelta());
	rval->SetAutomaticInitialSimplex(this->GetAutomaticInitialSimplex());

	return loPtr;
}

I tested it in my local copy and it worked as expected. I have a local copy but cannot push it to the repository. Even pushing to a fork doesnt work.

Is this something I can contribute or do you need more information?
Do I have to follow this guide for contribution: itk.org/Wiki/ITK/Git/Develop?


(Matt McCormick) #3

Hi @Gordian,

It sounds like you are on the right path :+1:.

We need to implement InternalClone explicitly, per-class since we need to differentiate between state that is part of the parameters of the class and state that is for internal computations, etc.

The best approach is to add the InternalClone method to the class directly so another class does not need to be maintained and so the community can benefit and contribute.

The next steps are to add the new functionality to the regression tests and contribute it upstream. For more information on the current contribution process, see the ITK/CONTRIBUTING.md file and the ITK Git Workflow section of the ITK Software Guide, Book 1. If you have any questions, please ask :slight_smile: .

Thanks,
Matt


(Gordian) #4

HI @matt.mccormick.

at this point the InternalClone() is only implemented for seemingly arbitrary classes. The use of this function differs from implementation to implementation. It seems that if this function is implemented it is more like a FIX than a feature because it is mostly implemented for all the parameters. Even the ones that are inherited from the parent class instead of implementing an InternalClone() method in the parent class.

Before I suggest more changes to this method I would like to discuss what exactly you have in mind that should be achieved by this method:

  1. Should it provide an complete clone of the object including all set parameters like input image pointers, variable members and oberservers?

or

  1. should it only cover the member variables that can be set by Get/Set methods?
    What should be excluded from the clone process?

  2. Concerning your remark of the states. Hence the member for internal computations are computed when the SetX() methods are called, I would not clone those internal members.

  3. I propose that only the members are cloned that are part of the class itself. Every inherited member should be cloned in the associated parent class. This means if the InternalClone() method is needed all the parent classes without this method should be changed. That would lead to a more consistent behavior and no need for copying all the member variables over and over again. Could this lead to problems regarding the consistency of the cloned objects w.r.t. what is copied in the parent classes or other itk internal behaviors like the Modified() or internal computations?

I am looking forward to our thoughts.
Thanks,
Gordian


(Matt McCormick) #5

Hi @Gordian,

Yes, most implementations of InternalClone() should start by calling Superclass::InternalClone() to avoid of copying the parent class members.

Your implementations in the second post are looking good. We want to copy state such as the transform parameters for transform classes or the Scales and InitialPosition for optimizer classes.

There are some cases, for example itk::Object::SetObjectName, where we do not want to copy the state of the SetX member to the clone, so I do not think we can use macros to make InternalClone automatic.

I hope this helps clarify things.

Matt


(Gordian) #6

Hi,

what I want to achieve is to have a vector with different ImageToImageMetrics. I create a vector that hold pointers to ImageToImageMetrics. I add some metrics and clone some metrics.

All classes have this macro. It returns a clone of the object it is called upon given that internal clone is written in the way of the posts above.

#define itkCloneMacro(x) \
  Pointer Clone() const    \
  {                        \
    Pointer rval = dynamic_cast<x *>(this->InternalClone().GetPointer()); \
    return rval;             \
  }

The code below is the way I try to use the cloned method:

#include "itkImage.h"
#include "itkImageToImageMetric.h"
#include "itkMeanSquaresImageToImageMetric.h"
#include "itkMattesMutualInformationImageToImageMetric.h"

static constexpr unsigned int Dim3D = 3;
using ImageType = itk::Image<float, Dim3D>;
using MetricBaseType = itk::ImageToImageMetric<ImageType, ImageType>;
using MetricBasePointer = MetricBaseType*;


int main(int argc, char* argv[])
{
	// Rigide 3D Transformation
	using MSMetricType = itk::MeanSquaresImageToImageMetric<ImageType, ImageType>;
	auto  MSMetric = MSMetricType::New();

	// Affine 3D Transformation
	using MIMetricType = itk::MattesMutualInformationImageToImageMetric<ImageType, ImageType>;
	auto MIMetric = MIMetricType::New();

	std::vector<MetricBasePointer> m_MetricVector;
	m_MetricVector.push_back(MSMetric);
	m_MetricVector.push_back(MIMetric);
	m_MetricVector.push_back(nullptr);
	m_MetricVector.push_back(nullptr);

	// fill the vector positions that hold a nullptr
	m_MetricVector.at(2) = m_MetricVector.at(0)->Clone();
	m_MetricVector.at(3) = m_MetricVector.at(0)->Clone();
	

	for (auto i = 0; i < m_MetricVector.size(); ++i)
	{
		std::cout << i << ". " << m_MetricVector.at(i)->GetNameOfClass() << std::endl;
	}

	return EXIT_SUCCESS;

}

After this i cannot access the cloned object through the vector interface ( .at(i) or [i] ). The objects are sliced to the base class. If I have a look at them they are turned into the base classes. If I try using the Print() methods I get an error for the cloned objects. The first two objects are fine as long as the pointer is not empty.

I guess the cloned elements are sliced to the base class but even virtual functions like GetNameOfClass() are not working.
How can I store different itk::SmartPointer in the same vector (like std:.vector<itk::ImageToImageMetric>> and can use them as the objects they actually are ( and not MetricBaseType*)?

I am using a vector of different metric and i do not know at compile time what element will have what type. Therfore, casting seems not be possible. The line below would not work.

std::cout << dynamic_cast<MIMetricType*>m_MetricVector.at(i)->GetNameOfClass() << std::endl;

Thanks in advance,
Gordian


(Gordian) #7

Why should the function return a LightObject::Pointer instead of an pointer of the object.
For example:

Self::Pointer
Optimizer
::InternalClone() const
{
	// Default implementation just copies the parameters from
	// this to new optimizer.
	Superclass::Pointer loPtr = Superclass::InternalClone();
	Self::Pointer rval = dynamic_cast<Self *>(loPtr.GetPointer());
	if (rval.IsNull())
	{
		itkExceptionMacro(<< "downcast to type " << this->GetNameOfClass() << " failed.");
	}

	rval->SetScales(this->GetScales());
	rval->SetInitialPosition(this->GetInitialPosition());
	return rval;
}

Which problems could arise?


(Matt McCormick) #8

This will be problematic in your use case: std::vector<MetricBasePointer> m_MetricVector;. A smart pointer should be used so a reference is kept to the object. That is,

using MetricBasePointer = MetricBaseType::Pointer;

After fixing the issue identified above, what is the output of this?

The function has the same function signature for polymorphism.


(Gordian) #9

Using your suggestion i changed this code to:

#include "itkImage.h"
#include "itkImageToImageMetric.h"
#include "itkMeanSquaresImageToImageMetric.h"
#include "itkMattesMutualInformationImageToImageMetric.h"

static constexpr unsigned int Dim3D = 3;
using ImageType = itk::Image<float, Dim3D>;
using MetricBaseType = itk::ImageToImageMetric<ImageType, ImageType>;
using MetricBasePointer = MetricBaseType::Pointer;

int main(int argc, char* argv[])
{
	// Rigide 3D Transformation
	using MSMetricType = itk::MeanSquaresImageToImageMetric<ImageType, ImageType>;
	auto  MSMetric = MSMetricType::New();

	// Affine 3D Transformation
	using MIMetricType = itk::MattesMutualInformationImageToImageMetric<ImageType, ImageType>;
	auto MIMetric = MIMetricType::New();

	std::vector<MetricBasePointer> m_MetricVector;
	m_MetricVector.push_back(MSMetric.GetPointer());
	m_MetricVector.push_back(MIMetric.GetPointer());
	m_MetricVector.push_back(nullptr);
	m_MetricVector.push_back(nullptr);

	// fill the vector positions that hold a nullptr
	m_MetricVector.at(2) = m_MetricVector.at(0)->Clone();
	m_MetricVector.at(3) = m_MetricVector.at(0)->Clone();
	
	for (auto i = 0; i < m_MetricVector.size(); ++i)
	{
		std::cout << i << ". " << m_MetricVector.at(i)->GetNameOfClass() << std::endl;
	}

	return EXIT_SUCCESS;

}

Now the output is:

0. MeanSquaresImageToImageMetric
1. MattesMutualInformationImageToImageMetric
2. MeanSquaresImageToImageMetric
3. MeanSquaresImageToImageMetric

That looks like the desired output. Also the variables are now correct. Thanks to @matt.mccormick for this suggestion.