Add quantitative tests to `itk::FastMarchingImageFilterBase`

[jhlegarreta]

Description

As mentioned in PR #114, the itk::FastMarchingFilterBase class would be better covered if the corresponding tests were quantitative (i.e. the output was compared against a baseline), and additional test cases were included.

Expected behavior

Improvement in the robustness of the class by:

• Making the tests quantitative adding some trial points.
• Testing non-identity spacing images.
• Testing on non-null origin images.

Actual behavior

No comparison against a baseline is done.
Default identity spacing and null origin images tested.

Reproducibility

%100.

Versions

ITK master.

Additional Information

This should be relatively easy to address.

[ignaciobartol]

Hi everyone, I am taking a look into this issue. This is my first time contributing to ITK, so it might take me a bit of time to implement the test effectively. I will update my progress throughout the day.

[ignaciobartol]

Hi, I've been taking a look at this today and I am a bit stuck with this issue as I am not sure if the modifications to the tests that I am planning on doing are aligned what is required in this issue.

I was thinking something along these lines:

@@ -234,6 +235,37 @@ itkFastMarchingTest(int argc, char * argv[])
    }
  }

+  {
+    auto nonIdentitySpacingValue = 2.0; // Non-identity spacing
+    auto nonIdentitySpacing = itk::MakeFilled<typename FloatFMType::OutputSpacingType>(nonIdentitySpacingValue);
+    marcher->SetOutputSpacing(nonIdentitySpacing);
+    ITK_TEST_SET_GET_VALUE(nonIdentitySpacing, marcher->GetOutputSpacing());
+
+    marcher->Update();
+
+    itk::ImageRegionIterator<FloatImage> nonIdentityIterator(marcher->GetOutput(), marcher->GetOutput()->GetBufferedRegion());
+    for (; !nonIdentityIterator.IsAtEnd(); ++nonIdentityIterator)
+    {
+      auto nonIdentityIndex = nonIdentityIterator.GetIndex();
+      nonIdentityIndex -= offset0;
+      double                distance;
+      distance = 0.0;
+      for (int j = 0; j < 2; ++j)
+      {
+        distance += nonIdentityIndex[j] * nonIdentityIndex[j];
+      }
+      distance = std::sqrt(distance) * nonIdentitySpacingValue;
+      auto outputValue = nonIdentityIterator.Get() * marcher->GetOutputSpacing().GetElement(0);
+      if (itk::Math::abs(outputValue) / distance > 1.42)
+      {
+        std::cout << nonIdentityIndex << ' ';
+        std::cout << itk::Math::abs(outputValue) / distance << ' ';
+        std::cout << itk::Math::abs(outputValue) << ' ' << distance << std::endl;
+        passed = false;

Likewise, I am planning on making similar changes for the rest of the tests and by also setting a different non-null origin. However, I am not sure if these changes align with what is expected. I might be way off track, so any hints or guidance are greatly appreciated!

Thank you!

[dzenanz]

Here is a comprehensive example of adding a additional regression testing:
#4330
This adds 3D regression test, where only 2D regression testing existed.

Here is another type of quantitative test:
#4254

This is the simplest possible quantitative test:
#1793

Here is relatively simple image-based regression test addition:
#315
If this style can be applied here, this is probably the easiest.

[dzenanz]

I was thinking something along these lines

This is probably closest to options 2 and 3 from my previous comment. I did not look deeply into this.

[ignaciobartol]

Here is a comprehensive example of adding a additional regression testing: #4330 This adds 3D regression test, where only 2D regression testing existed.

Here is another type of quantitative test: #4254

This is the simplest possible quantitative test: #1793

Here is relatively simple image-based regression test addition: #315 If this style can be applied here, this is probably the easiest.

Thank you! Sounds good, I'll take a look into those PRs and try to formulate the tests to better cover this module.

[jhlegarreta]

👍 Please, define the 1.42 threshold in your example as a variable and add a comment on why that value is reasonable.