Pull request 52 (ReducedFullSampler) updated

[N-Dekker]

• Brought pull request ADD: Added the ReducedFullSampler #52 from @MathiasPolfliet up-to-date with the current revision of the main branch.

---

Other pull requests from Mathias, currently at https://github.com/SuperElastix/elastix/pulls?q=is%3Apr+is%3Aclosed+author%3AMathiasPolfliet+:

• pull request 51: ENH: Added more functionality to the stacktransform #51
• pull request 56: ENH: Added functionality related to the spatial Jacobian to the StackTransform #56 (appears to have the same code changes as pull request 51)
• pull request 60: ADD: Added the StackTransformBendingEnergy #60

The code of the original pull requests was locally retrieved by git fetch origin pull/ID/head:BRANCH_NAME, as described at GitHub Docs - Checking out pull requests locally. The other two new branches based on those old pull request are:

• https://github.com/SuperElastix/elastix/tree/MathiasPolfliet/More-functionality-stacktransform
• https://github.com/SuperElastix/elastix/tree/MathiasPolfliet/StackTransformBendingEnergy

[N-Dekker]

This appears to be the essential part, compared to the "non-reduced" FullSampler. Looks like seven lines of code could be simplified to the following three:

    InputImageRegionType region = this->GetCroppedInputImageRegion();
    region.SetIndex(ReducedInputImageDimension, 0);
    region.SetSize(ReducedInputImageDimension, 1);

[N-Dekker]

@MathiasPolfliet @mstaring @stefanklein Is it really necessary for the "reduced" sampler to do index[ReducedInputImageDimension] = 0? Or would it be sufficient to only adjust the size, and leave the index alone?

I'm asking, because ITK allows specifying start index of the buffered region of an image to be non-zero. So an index value of zero may be outside the buffered region. Trying to retrieve samples outside the buffered region might cause crashes. So if it is sufficient for the "reduced" sampler to only adjust the size, I would prefer to leave the index alone.

Another option might be something like:

// Instead of index[ReducedInputImageDimension] = 0
index[ReducedInputImageDimension] = bufferedRegion.GetIndex(ReducedInputImageDimension);

But I wonder if that's useful/necessary.

[N-Dekker]

Comparing ReducedFullSampler and FullSampler, based on the original code at commit 2ec22d0 "VER: increased to 4.900 in preparation of release" (2018):

elxReducedFullSampler.h vs elxFullSampler.h

• 124 vs 123 lines
• Added ReducedInputImageDimension (unnecessary in this case, it seems)

elxReducedFullSampler.h vs elxFullSampler.h

• 27 vs 32 lines
• no relevant changes (just "nothing")

elxReducedFullSampler.cxx vs elxFullSampler.cxx

• 18 vs 22 lines
• no relevant changes (just elxInstallMacro)

itkReducedFullSampler.h vs itkFullSampler.h

• 124 vs 126 lines
• Added ReducedInputImageDimension and InputImageSizeType
• No ThreadedGenerateData

itkReducedFullSampler.hxx vs itkFullSampler.hxx

• 150 vs 246 lines
• No ThreadedGenerateData
• Essential added code at the begin of GenerateData:

  InputImageIndexType index = this->GetCroppedInputImageRegion().GetIndex();
  index[ ReducedInputImageDimension ] = 0;
  InputImageSizeType size = this->GetCroppedInputImageRegion().GetSize();
  size[ ReducedInputImageDimension ] = 1;
  InputImageRegionType region;
  region.SetIndex( index );
  region.SetSize( size );
  InputImageIterator iter( inputImage, region );

[N-Dekker]

Some comments:

• There appears a lot of code duplication between this component and FullSampler. Can't ReducedFullSampler be written in terms of FullSampler? The FullSampler has very much been improved recently (commit b7029ed, 4d028f7 etc.). It would have been a pity if those improvements won't be included with ReducedFullSampler.
• I would rename it "ReducedDimensionFullSampler", on order to be consistent with the existing "ReducedDimensionBSplineInterpolator". OK?
• Does this suggest a reduced-dimension version for Grid, Random, RandomCoordinate, and RandomSparseMask, as well?

@MathiasPolfliet @mstaring @stefanklein Instead of this proposed new component, "ReducedFullSampler" wouldn't it be better to just add support for a new parameter, (ReduceDimension "true"), to the existing FullSampler? And possibly also to other existing sampler components?