Skip to content

Add streaming base classes and streaming statistics#855

Merged
blowekamp merged 16 commits intoInsightSoftwareConsortium:masterfrom
blowekamp:AddStreamingBaseClasses
May 16, 2019
Merged

Add streaming base classes and streaming statistics#855
blowekamp merged 16 commits intoInsightSoftwareConsortium:masterfrom
blowekamp:AddStreamingBaseClasses

Conversation

@blowekamp
Copy link
Copy Markdown
Member

No description provided.

@blowekamp blowekamp changed the title WIP: Add streaming base classes WIP: Add streaming base classes and streaming statistics May 10, 2019
@blowekamp
Copy link
Copy Markdown
Member Author

Changing the base class for the StatisticsFilters to the ImageSink with streaming support when very smoothly. More testing and documentation passes are still needed.

Please review the style interface for the StreamingProcessObject and ImageSink classes.

A backwards incompatibility change introduced is that the LabelStatisticsImageFilter and StatisticsImageFilter no longer produce an image as output since they are no longer derived from ImageToImageFilter.

Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Comment thread Modules/Core/Common/include/itkStreamingProcessObject.h Outdated
Comment thread Modules/Core/Common/include/itkStreamingProcessObject.h Outdated
Comment thread Modules/Core/Common/include/itkStreamingProcessObject.h Outdated
Comment thread Modules/Core/Common/src/itkStreamingProcessObject.cxx Outdated
Comment thread Modules/Core/Common/include/itkImageSink.hxx Outdated
@blowekamp blowekamp force-pushed the AddStreamingBaseClasses branch 5 times, most recently from 85e2edd to d12d5c0 Compare May 14, 2019 19:24
Comment thread Documentation/ITK5MigrationGuide.md Outdated
Comment thread Modules/Core/Common/include/itkImageSink.hxx
@blowekamp blowekamp requested a review from zivy May 14, 2019 19:33
Comment thread Modules/Filtering/ImageStatistics/test/itkStatisticsImageFilterTest.cxx Outdated
Comment thread Modules/Numerics/Statistics/include/itkImageToHistogramFilter.h Outdated
@dzenanz
Copy link
Copy Markdown
Member

dzenanz commented May 14, 2019

Looking good so far.

@blowekamp
Copy link
Copy Markdown
Member Author

I have completed refactoring the classes. I'll write some detailed/corner cases tomorrow to wrap things up.

@romangrothausmann Please check out this PR and test the classes. Thank you!

Copy link
Copy Markdown
Member

@zivy zivy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments in line. Still a review in process (will continue tomorrow).

Comment thread Modules/Core/Common/include/itkImageSink.h Outdated
Comment thread Modules/Core/Common/include/itkImageSink.h
Comment thread Modules/Core/Common/include/itkImageSink.hxx Outdated
Comment thread Modules/Core/Common/include/itkImageSink.hxx
Comment thread Modules/Core/Common/include/itkImageSink.hxx
Comment thread Modules/Core/Common/include/itkStreamingProcessObject.h Outdated
@romangrothausmann
Copy link
Copy Markdown
Member

Many thanks @blowekamp, this is a very great contribution! Your former module for ITK4 has helped us a lot when we were in the need to get statistics of very large datasets and we worried about its missing compatibility with the transition to ITK-5. With this PR this would be solved and its future maintenance ensured. In addition it is great that this PR also includes the ideas of blowekamp/itkStreamingSinc#3.

Copy link
Copy Markdown
Member

@zivy zivy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added some more comments / questions. Nothing that is too critical.

Comment thread Modules/Core/Common/include/itkImageToImageFilter.h
Comment thread Modules/Core/Common/include/itkStreamingProcessObject.h Outdated
Comment thread Modules/Core/Common/src/itkStreamingProcessObject.cxx Outdated
Comment thread Modules/Core/Common/src/itkStreamingProcessObject.cxx
Comment thread Modules/Core/Common/src/itkStreamingProcessObject.cxx
Comment thread Modules/Core/Common/src/itkStreamingProcessObject.cxx
Comment thread Modules/Core/Common/src/itkStreamingProcessObject.cxx Outdated
Comment thread Modules/Filtering/ImageFeature/include/itkBilateralImageFilter.hxx
Comment thread Modules/Filtering/ImageStatistics/include/itkStatisticsImageFilter.hxx Outdated
@blowekamp blowekamp force-pushed the AddStreamingBaseClasses branch from d12d5c0 to 87f4327 Compare May 15, 2019 16:12
blowekamp added 2 commits May 15, 2019 13:56
The StreamingProcesObject provide a pipeline mechanics to update the
input's pipeline with a sequence of regions and generically execute
them in a StreamedGenerateData method.
Adding the ImageSink base class which provides support for streaming
and multi-threading on an input image. Generally this is used when
transforming an image into something like computing statistics, or a
RLE of an image.
@blowekamp blowekamp force-pushed the AddStreamingBaseClasses branch from 87f4327 to d44a529 Compare May 15, 2019 18:29
@blowekamp blowekamp changed the title WIP: Add streaming base classes and streaming statistics Add streaming base classes and streaming statistics May 15, 2019
blowekamp added 3 commits May 15, 2019 14:34
Add functionality, to compute the label statistics of labeled image in
a streamed or out-of-core way. For example, from a very large
segmented image the bounding boxes can be computed to determine a
region of interest to load.
blowekamp added 7 commits May 15, 2019 14:34
And the same change from ImageToImgeFilter to verify that input images
are congruent in physical space by default.
Change the base class from ImageTransformer to ImageSink and expose
SetNumberOfStreamDivisions method to enable streamed generation of
Histograms. Also updated the way decorated input are created to
automatically create the decorator inside the Set method.

If the automatic computation of the minimum and maximum is enabled
then streaming is disabled by the overloaded
GetNumberOfInputRequestedRegions method.
The ImageSinc can be used as a replacement.
Use the double graft best practice for mini-pipelines in the
NormalizeImageFilter.
@blowekamp blowekamp force-pushed the AddStreamingBaseClasses branch from d44a529 to a17bb6e Compare May 15, 2019 18:35
@blowekamp blowekamp force-pushed the AddStreamingBaseClasses branch from a17bb6e to 23da91e Compare May 16, 2019 14:12
@blowekamp
Copy link
Copy Markdown
Member Author

This patch should be ready to go. I'm just refreshing the CI after one VNL test failure due to time profiling.

@romangrothausmann
Copy link
Copy Markdown
Member

Trying to test this with my ITK-CLIs and am hitting the backwards incompatibility that the StatisticsImageFilter no longer produce an image as output. What would be the recommended way to adjust the code accordingly? Basically I would replace e.g.

stat->SetInput(extractor->GetOutput());
adder->SetInput1(stat->GetOutput());
adder->Update();
std::cerr << stat->GetMaximum();

by

stat->SetInput(extractor->GetOutput());
adder->SetInput1(extractor->GetOutput());
adder->Update();
stat->Update();
std::cerr << stat->GetMaximum();

Is there a more elegant way to trigger stat to update if adder is updated, similar to how it was possible before?

@blowekamp
Copy link
Copy Markdown
Member Author

Great, thank you for testing!

I take it there is no streaming involved here. All you should have to do is simple call stat->Update(); seems elegant to me. 😎

@romangrothausmann
Copy link
Copy Markdown
Member

romangrothausmann commented May 16, 2019

Thanks for the feedback. No, no streaming there. I'll just use the additional stat->Update();
I am hitting this though with Module_ParabolicMorphology=ON:

/opt/itk/include/ITK-5.0/itkParabolicOpenCloseSafeBorderImageFilter.hxx:48:48: error: no matching function for call to 'itk::StatisticsImageFilter<itk::Image<unsigned char, 3u> >::GetOutput()'
     typename TInputImage::SpacingType spcing = m_StatsFilt->GetOutput()->GetSpacing();

That originates from:
https://github.com/InsightSoftwareConsortium/ITKParabolicMorphology/blob/d4bc69362c3e435ff5d97ed5bee0167199da69ac/include/itkParabolicOpenCloseSafeBorderImageFilter.hxx#L48

@blowekamp
Copy link
Copy Markdown
Member Author

Here is a PR to improve that filter: InsightSoftwareConsortium/ITKParabolicMorphology#26

@blowekamp blowekamp merged commit c76b193 into InsightSoftwareConsortium:master May 16, 2019
romangrothausmann added a commit to romangrothausmann/ITK that referenced this pull request May 17, 2019
romangrothausmann added a commit to romangrothausmann/ITK that referenced this pull request May 17, 2019
romangrothausmann added a commit to romangrothausmann/ITK that referenced this pull request May 17, 2019
@romangrothausmann
Copy link
Copy Markdown
Member

@blowekamp I'm getting warnings like:

WARNING: In /opt/itk/include/ITK-5.0/itkImageSink.hxx, line 88
ImageToHistogramFilter (0x2955840): Unable to convert input "HistogramBinMaximum" to type N3itk5ImageIfLj3EEE

when using ImageToHistogramFilter of this PR with my hist see e.g. the log here for SDI:
https://gitlab.com/romangrothausmann/ITK-CLIs/-/jobs/214553760
and also if not using SDI:
https://gitlab.com/romangrothausmann/ITK-CLIs/-/jobs/214553759
It is unclear to my why and what to do about them. The result is still the same though.

@hjmjohnson
Copy link
Copy Markdown
Member

@blowekamp I am also getting new warnings for every run of my application:

WARNING: In /Shared/johnsonhj/Binaries/20190513/Linux/NEP-14/ITKv5/Modules/Core/Common/include/itkImageSink.hxx, line 88
LabelStatisticsImageFilter (0x278c2d0): Unable to convert input "LabelInput" to type N3itk5ImageIdLj3EEE

WARNING: In /Shared/johnsonhj/Binaries/20190513/Linux/NEP-14/ITKv5/Modules/Core/Common/include/itkImageSink.hxx, line 88
ImageToHistogramFilter (0x279bd70): Unable to convert input "AutoMinimumMaximum" to type N3itk5ImageIsLj3EEE

WARNING: In /Shared/johnsonhj/Binaries/20190513/Linux/NEP-14/ITKv5/Modules/Core/Common/include/itkImageSink.hxx, line 88
ImageToHistogramFilter (0x279bd70): Unable to convert input "HistogramSize" to type N3itk5ImageIsLj3EEE

WARNING: In /Shared/johnsonhj/Binaries/20190513/Linux/NEP-14/ITKv5/Modules/Core/Common/include/itkImageSink.hxx, line 88
ImageToHistogramFilter (0x279bd70): Unable to convert input "MarginalScale" to type N3itk5ImageIsLj3EEE

N-Dekker added a commit to N-Dekker/ITKTools that referenced this pull request Jan 26, 2024
`StatisticsImageFilter` is no longer an `ImageToImageFilter`, so it no longer supports `GetOutput()`, from pull request InsightSoftwareConsortium/ITK#855
InsightSoftwareConsortium/ITK@0efe8db "ENH: Add streaming support to StatisticsImageFilter", May 15, 2019
N-Dekker added a commit to N-Dekker/ITKTools that referenced this pull request Jan 26, 2024
`StatisticsImageFilter` is no longer an `ImageToImageFilter`, so it no longer supports `GetOutput()`, from pull request InsightSoftwareConsortium/ITK#855
InsightSoftwareConsortium/ITK@0efe8db "ENH: Add streaming support to StatisticsImageFilter", May 15, 2019
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
…reamingBaseClasses

Add streaming base classes and streaming statistics
hjmjohnson pushed a commit to hjmjohnson/ITK that referenced this pull request May 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants