WIP: streaming for resample image filter in case of linear transforms

[romangrothausmann]

Inspired by the discussion of the discourse topic Why ResampleImageFilter is slow?, this PR is a try to realize streaming for at least some transforms for the itkResampleImageFilter.
It is one of the most general filters in ITK and much used and the essential one for isotropic down-sampling of large datasets. Especially for that case it would be very helpful to have streaming capabilities, in order to generate smaller datasets for filters that cannot stream. Apart from a reduced memory footprint, streaming can lead to a significant speed up because a chunk can be processed while the next is loaded from disc, drastically reducing loading latencies.

As the filter is of great importance, the idea is to first create some new test cases (failing with the current implementation) that are expected to succeed (partially) when streaming is realized. These tests are now merged from #392 and ready committed to the branch test_RSI-SDI (based on v4.13.1) and merged into streaming4ResampleImageFilter for testing the new contributions to itkResampleImageFilter, which enable streaming for linear transforms. Test showed that streaming already works but (even for an identity transform) the result's extent is larger than without streaming.

Therefore, itkResampleImageTest7 is meant to compare the current results of itkResampleImageFilter in various cases with those obtained when streaming is employed. There, a reader and a writer are avoided to rule out any interference concerning streaming limitations in respect to a chosen file format.
itkResampleImageTest2s is based on itkResampleImageTest2 adjusted to write MHAs, which support streaming.

[jhlegarreta]

@romangrothausmann

Thanks for the patches and the discussion !

I am not an expert in the way these filters were implemented, but others will hopefully chime in.

A few comments that also apply to #84 :

• The transition to GitHub is still note completed, it might take a few days or weeks still. Meanwhile, we still use gerrit.
• You may want to read the coding style section (Appendix 3) of the ITK SW Guide. Specifically, and although some residuals may still be around in the SW Guide or remote parts of the code:
  • We no longer use the keyword typedef, but use the C++11 friendly using. You may want to visit http://review.source.kitware.com/#/c/23103/ to see why.
  • Rather than using try / catch blocks, we have the TRY_EXPECT_NO_EXCEPTION macro that should be used in tests for the sake of readability. IT makes the code shorter also.
  • Other conventions (the use of the auto keyword, range-based loops, the use of nullptr, the override keyword, use of C++11 headers, etc.) may need to be applied to the code you contributed with also.

The above is not an exhaustive list. Hopefully, the ITK v5 migration guide provides further insight. Also you may find many useful scripts here to do the job for you.

Also, do not forget to stick to the commit message convention (ENH, DOC, etc.). Some of the commits are missing it.

Thanks !

[romangrothausmann]

Thanks @jhlegarreta for looking at this and your comments.
I created a PR because @blowekamp asked for one, PR meant for me GH not gerrit (does gerrit have PRs?). Also, I'm waiting for GH because I'm not so familiar with gerrit (and the contribution is likely going to take some time before it is ready for a merge...).
It is not clear to me what code style changes are specific for a contribution to ITK-5 and which expected for a contribution to ITK-4.13.1. Mine here is specifically based on ITK-4 because I would need it for a project that is not (yet) ported to ITK-5. When it works for ITK-4 I would port it to ITK-5 and open a new PR.
Concerning the "commit message convention": I sometimes have difficulties to assign one of the given categories to a commit, e.g. what would be the right one for 0f962c5? ENH, BUG or all together WIP? In the end, I might consider squashing all commits into the first working commit (then using ENH) or is that not preferred?

[blowekamp]

Thanks for creating the PR to share your code. I hope to get to this tomorrow afternoon. Thank you for you patience.

[romangrothausmann]

Thanks for creating the PR to share your code. I hope to get to this tomorrow afternoon. Thank you for you patience.

Sure, no problem. The whole contribution is going to take some more time anyway and I am very thankful for any help and advice on this, especially since I am a bit lost on the problem with itkResampleImageTest7 and the change in output extent when streaming is employed.

[jhlegarreta]

It is not clear to me what code style changes are specific for a contribution to ITK-5 and which expected for a contribution to ITK-4.13.1. Mine here is specifically based on ITK-4 because I would need it for a project that is not (yet) ported to ITK-5. When it works for ITK-4 I would port it to ITK-5 and open a new PR.

@romangrothausmann, true, you are right: I had completely neglected the intention to submit this to ITK 4.13.

Some of the comments concerning the use of macros (e.g. TRY_EXPECT_NO_EXCEPTION) are still valid, though.

But your roadmap sounds reasonable, yes ! Thanks for the explanation !

Concerning the "commit message convention": I sometimes have difficulties to assign one of the given categories to a commit, e.g. what would be the right one for 0f962c5? ENH, BUG or all together WIP? In the end, I might consider squashing all commits into the first working commit (then using ENH) or is that not preferred?

True, at times it is difficult to tell, especially when there are many changes, and the payload of making all of them separately is too high.

Squashing would be OK I guess.

[thewtex]

@romangrothausmann could this please be rebased on master?

[romangrothausmann]

Sure. Rebased, adjusted and squashed (like for #392).

The individual commits are in https://github.com/romangrothausmann/ITK/commits/streaming4ResampleImageFilter_incr, in particular with romangrothausmann@2cdc0ee the streaming tests are now expected to pass.

[romangrothausmann]

in particular with romangrothausmann/ITK@2cdc0ee the streaming tests are now expected to pass.

Which they do not.
In ResampleImageTest7 the monitor filter reports:
Down stream filter didn't execute PropagateRequestedRegion well
which could hint towards the problem I was experiencing before (https://discourse.itk.org/t/why-resampleimagefilter-is-slow/1217/17?u=grothausmann.roman), i.e. that the output extent is different with streaming.

[romangrothausmann]

Just noticed that I can't change this PR to be for master now (not v4.13.1 as originally intended).
Therefore reset to old state and created a new PR #469.

Should this PR closed in favor of #469?

[thewtex]

@romangrothausmann excellent. Yes, we can close this PR in favor of #469.