Skip to content

Commit

Permalink
ENH: Throw exception if "NumberOfResolutions" parameter has value zero
Browse files Browse the repository at this point in the history
Distinguished clearly between zero and one. In this case, the value zero must be a mistake.

Improved consistency between fixed and moving pyramid.

Discussed at our weekly internal elastix sprint meeting.
  • Loading branch information
N-Dekker committed Mar 27, 2023
1 parent fe2b051 commit 9b2b22c
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@ MultiMetricMultiResolutionRegistration<TElastix>::BeforeRegistration()
/** Set the number of resolutions.*/
unsigned int numberOfResolutions = 3;
this->m_Configuration->ReadParameter(numberOfResolutions, "NumberOfResolutions", 0);
if (numberOfResolutions == 0)
{
itkExceptionMacro("The NumberOfResolutions parameter must have a non-zero value!");
}

this->SetNumberOfLevels(numberOfResolutions);

/** Set the FixedImageRegions to the buffered regions. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ MultiResolutionRegistration<TElastix>::BeforeRegistration()
/** Set the number of resolutions. */
unsigned int numberOfResolutions = 3;
this->m_Configuration->ReadParameter(numberOfResolutions, "NumberOfResolutions", 0);
if (numberOfResolutions == 0)
{
itkExceptionMacro("The NumberOfResolutions parameter must have a non-zero value!");
}

this->SetNumberOfLevels(numberOfResolutions);

/** Set the FixedImageRegion. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ MultiResolutionRegistrationWithFeatures<TElastix>::BeforeRegistration()
/** Set the number of resolutions. */
unsigned int numberOfResolutions = 3;
this->m_Configuration->ReadParameter(numberOfResolutions, "NumberOfResolutions", 0);
if (numberOfResolutions == 0)
{
itkExceptionMacro("The NumberOfResolutions parameter must have a non-zero value!");
}
this->SetNumberOfLevels(numberOfResolutions);

/** Set the FixedImageRegions to the buffered regions. */
Expand Down
2 changes: 1 addition & 1 deletion Components/elxGenericPyramidHelper.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class GenericPyramidHelper
configuration.ReadParameter(numberOfResolutions, "NumberOfResolutions", 0, true);
if (numberOfResolutions == 0)
{
numberOfResolutions = 1;
itkGenericExceptionMacro("The NumberOfResolutions parameter must have a non-zero value!");
}

/** Create a default schedule. Set the numberOfLevels first. */
Expand Down
2 changes: 1 addition & 1 deletion Core/ComponentBaseClasses/elxFixedImagePyramidBase.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ FixedImagePyramidBase<TElastix>::SetFixedSchedule()
configuration.ReadParameter(numberOfResolutions, "NumberOfResolutions", 0, true);
if (numberOfResolutions == 0)
{
numberOfResolutions = 1;
itkExceptionMacro("The NumberOfResolutions parameter must have a non-zero value!");
}

/** Create a default schedule. Set the numberOfLevels first. */
Expand Down
3 changes: 1 addition & 2 deletions Core/ComponentBaseClasses/elxMovingImagePyramidBase.hxx
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,8 @@ MovingImagePyramidBase<TElastix>::SetMovingSchedule()
configuration.ReadParameter(numberOfResolutions, "NumberOfResolutions", 0, true);
if (numberOfResolutions == 0)
{
log::error("ERROR: NumberOfResolutions not specified!");
itkExceptionMacro("The NumberOfResolutions parameter must have a non-zero value!");
}
/** \todo quit program? Actually this check should be in the ::BeforeAll() method. */

/** Create a default schedule. Set the numberOfLevels first. */
this->GetAsITKBaseType()->SetNumberOfLevels(numberOfResolutions);
Expand Down
41 changes: 41 additions & 0 deletions Core/Main/GTesting/itkElastixRegistrationMethodGTest.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,47 @@ GTEST_TEST(itkElastixRegistrationMethod, IsDefaultInitialized)
}


// Tests that the value zero is rejected for the "NumberOfResolutions" parameter.
GTEST_TEST(itkElastixRegistrationMethod, RejectZeroValueForNumberOfResolution)
{
constexpr auto ImageDimension = 2U;
using PixelType = float;
using ImageType = itk::Image<PixelType, ImageDimension>;
const itk::Size<ImageDimension> imageSize{ { 5, 6 } };

elx::DefaultConstruct<elx::ParameterObject> parameterObject{};
elx::DefaultConstruct<ElastixRegistrationMethodType<ImageType>> registration{};

registration.SetFixedImage(CreateImage<PixelType>(imageSize));
registration.SetMovingImage(CreateImage<PixelType>(imageSize));
registration.SetParameterObject(&parameterObject);

elx::ParameterObject::ParameterMapType parameterMap{ // Parameters in alphabetic order:
{ "ImageSampler", { "Full" } },
{ "MaximumNumberOfIterations", { "2" } },
{ "Metric", { "AdvancedNormalizedCorrelation" } },
{ "Optimizer", { "AdaptiveStochasticGradientDescent" } },
{ "Transform", { "TranslationTransform" } }
};

parameterObject.SetParameterMap(parameterMap);

// OK: "NumberOfResolutions" is unspecified, so use its default value.
EXPECT_NO_THROW(registration.Update());

for (const unsigned int numberOfResolutions : { 1, 2 })
{
// OK: Use a value greater than zero.
parameterMap["NumberOfResolutions"] = { std::to_string(numberOfResolutions) };
EXPECT_NO_THROW(registration.Update());
}

// Expected to be rejected: the value zero.
parameterMap["NumberOfResolutions"] = { "0" };
EXPECT_THROW(registration.Update(), itk::ExceptionObject);
}


// Tests registering two small (5x6) binary images, which are translated with respect to each other.
GTEST_TEST(itkElastixRegistrationMethod, Translation)
{
Expand Down

0 comments on commit 9b2b22c

Please sign in to comment.