ITKSphinxExamples icon indicating copy to clipboard operation
ITKSphinxExamples copied to clipboard

WIP: ENH: Add watershed with distance map example.

Open jhlegarreta opened this issue 7 years ago β€’ 29 comments

Add watershed with distance map example.

Resolves #48.

jhlegarreta avatar Dec 23 '18 21:12 jhlegarreta

Folks, thanks for your patience. Still WIP, though 😞 .

A few notes:

  • Although the Cxx version does compile and produce an output, it is not the expected one. So some help would be appreciated.
  • The Python counterpart has still other issues (comments in-line), since the code does not run from the beginning to the end.
  • Given that the expected outputs are not obtained yet, the ITKExamples site Documentation.rst is not yet finished.
  • The segmentation_result_analysis.py and segmentation_result_visualization.py are yet to be tested.

Finally, it looks like the original code is locally failing at the apply_watershed method due to some incorrect template parameter that I ignore where it comes from.

jhlegarreta avatar Dec 23 '18 21:12 jhlegarreta

@kmader you are welcome to review. Please.

jhlegarreta avatar Dec 23 '18 21:12 jhlegarreta

2ed8b81 rebased on master to start off from the wave of examples transitioned from the WikiExamples.

jhlegarreta avatar Jun 22 '19 02:06 jhlegarreta

85ca522 rebased on master to avoid conflicts early on after the last big example import from the WikiExamples has been merged.

jhlegarreta avatar Jun 28 '19 00:06 jhlegarreta

This is looking great :eyes:

thewtex avatar Mar 19 '20 21:03 thewtex

Not yet Matt. I think the few issues mentioned in this comment still remain, and I am still unable to run Kevin's original script to have a ground-truth to compare to.

jhlegarreta avatar Mar 19 '20 22:03 jhlegarreta

Check out this pull request onΒ  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

Rebased

thewtex avatar May 18 '21 12:05 thewtex

0a865e0: not finished yet/not tested/not formatted using black, but fixed a couple of obvious things in the method calls, and following @dzenanz 's comment in https://discourse.itk.org/t/runtimeerror-no-suitable-template-parameter-can-be-found/4159/13

  • Used the procedural interface in the object instantiation
  • Simplified the pixel type declaration by using enumeration types.
  • Used argparse following the convention introduced in PR #300.

jhlegarreta avatar Jun 25 '21 00:06 jhlegarreta

@thewtex looks like the GitHub lint action usingblack is formatting the code to a default width of 88: https://github.com/InsightSoftwareConsortium/ITKSphinxExamples/pull/86/checks?check_run_id=2947841475#step:7:88

I had formatted locally to 120 πŸ˜… , following what has been adopted de facto in ITK (cross referencing https://github.com/InsightSoftwareConsortium/ITK/pull/2608), which is also the length for the C++ code. Should a config file be added to set the length to 120 or are we keeping the default 79?

Thanks.

jhlegarreta avatar Jun 30 '21 01:06 jhlegarreta

I guess you can follow the example of https://github.com/InsightSoftwareConsortium/ITK/pull/2608, and use 120 character lines.

dzenanz avatar Jun 30 '21 13:06 dzenanz

I guess you can follow the example of InsightSoftwareConsortium/ITK#2608, and use 120 character lines.

The thing is that the lint check should be updated, or a toml config file be added with the chosen length, to have it passing.

jhlegarreta avatar Jun 30 '21 15:06 jhlegarreta

toml config file be added

Sounds easy enough πŸ˜„

dzenanz avatar Jun 30 '21 16:06 dzenanz

Adding pyproject.toml (e.g., this one: https://github.com/Leengit/ITK/blob/black_format/pyproject.toml) to the repository top-level directory will set black parameters such as line-length for this repository on your local computer, but I am unsure if or how it affects github actions.

@Leengit black is set to look for a pyproject.toml file automatically, without needing to set its location. So IIUC it should do the same anywhere, whether it is locally or inside a GitHub actions VM. This should be the case for the file you added to ITK if a Python linting action is to be added.

jhlegarreta avatar Jun 30 '21 17:06 jhlegarreta

Shorter line lengths may be needed to prevent content from being cut off in the rendered PDF.

thewtex avatar Jul 01 '21 16:07 thewtex

Shorter line lengths may be needed to prevent content from being cut off in the rendered PDF.

Not sure how the PDF generation would behave: e.g. the same should have happened to the ITK SW Guide when the C++ line length was set to 120 unless the ITK core examples folder was excluded and the files manually formatted; not sure if the VTK folks use clang-format (and if they do, which line length the use) in their VTKExamples repository, which AFAIK can be automatically put to a PDF.

But I've seen the agreement to adopt black's default 88 in the ITK core in the cross-referenced PR, so I will stick to that :+1:.

jhlegarreta avatar Jul 02 '21 13:07 jhlegarreta

Examples (which go into the software guide) have a different, lower line length limit: https://github.com/InsightSoftwareConsortium/ITK/blob/master/Examples/.clang-format#L75

dzenanz avatar Jul 02 '21 20:07 dzenanz

Tried to have a go at Kevin's script to be able to reproduce his result and hopefully complete the PR, I am getting the same error reported here: https://discourse.itk.org/t/runtimeerror-no-suitable-template-parameter-can-be-found/4159

I also tried using the CastImageFilter to cast from the output image type of the watershed image filter to itk.Image[itk.UL, 3], but that gives a similar error to the above saying that CastImageFilter is not wrapped for those types/the input types are not supported (cross-referencing ITK issue 2551).

Trying to specify the ttype=itk.Image[itk.UL, 3] parameter to itk.GetArrayFromImage() results in another error: TypeError: in method 'itkPyBufferIUL3__GetArrayViewFromImage', argument 1 of type 'itkImageUL3 *'

I do not feel like having the necessary bandwidth to build the wrapping from sources, so following ITK PR 2554, I will wait to the next release to give this another try. I do not see any other solution to this.

Thanks.

jhlegarreta avatar Jul 13 '21 02:07 jhlegarreta

Are you using self-built wrappings from master? Or pre-compiled, pip-installed wheels of version 5.2?

Edit: I wrote the question after reading the first paragraph 😞

dzenanz avatar Jul 13 '21 13:07 dzenanz

Waiting for the next release of Python wheels sounds reasonable.

dzenanz avatar Jul 13 '21 13:07 dzenanz

Are you using self-built wrappings from master? Or pre-compiled, pip-installed wheels of version 5.2?

I pip-installed 5.2.

Edit: I wrote the question after reading the first paragraph disappointed

:+1:

jhlegarreta avatar Jul 13 '21 13:07 jhlegarreta

Gave another go at this after ITK PR 2554 had been merged and shipped into ITK 5.2.1.post1:

  • A standalone Python version of the notebook works until the mesh grid creation step. I'd assume the notebook would work until that point. Not sure what's the issue there (the segmentation takes 1h30 so difficult to easily debug).
  • For the pure ITK-based code proposed in this PR, it looks like the default wrapped types will not be enough:
multiply_image_filter = itk.MultiplyImageFilter[
itk.support.extras.TemplateTypeError: 
itk.MultiplyImageFilter is not wrapped for input type `itk.Image[itk.UC,3], itk.Image[itk.UC,3], itk.Image[itk.F,3]`.

raised here.

I'd say that the idea is to avoid building ITK from source with all necessary types wrapped to have this working. Any suggestion @dzenanz @thewtex? Thanks.

jhlegarreta avatar Sep 06 '21 19:09 jhlegarreta

not wrapped for input type itk.Image[itk.UC,3], itk.Image[itk.UC,3], itk.Image[itk.F,3]

First use itk_cast_filter to convert UC to F, then do multiply with all 3 types of itk.F? Or multiply first with all 3 types of itk.UC, then cast to itk.F?

dzenanz avatar Sep 06 '21 19:09 dzenanz

First use itk_cast_filter to convert UC to F, then do multiply with all 3 types of itk.F? Or multiply first with all 3 types of itk.UC, then cast to itk.F?

:+1: Thanks @dzenanz. Gave it another go.

Looks closer than before, but still not fully working:

  • Writing directly the output of the watershed image filter instead of using an RGB color image (cross-referencing ITK issue 2551 @brad-t-moore).
  • The result is opposite to the expected one; the foreground/background values used or the reverse image need investigation.
  • The morphological operation on the watershed output image encounters the same difficulty about the image type not being wrapped.
  • (Will fix the format once the scripts work as expected.)

jhlegarreta avatar Sep 06 '21 22:09 jhlegarreta

@dzenanz thanks for spending a thought about this.

What is the first error message?

  File "/ITKEx/src/Segmentation/Watersheds/SegmentWithWatershedAndDistanceMap/Code.py", line 131, in <module>
    segmented_clean_image = itk.binary_morphological_opening_image_filter(watershed_image, kernel=structuring_element)
  File "{my_path}\.conda\envs\itkex\lib\site-packages\itk\support\helpers.py", line 173, in image_filter_wrapper
    return image_filter(*args, **kwargs)
  File "{my_path}\.conda\envs\itkex\lib\site-packages\itk\itkBinaryMorphologicalOpeningImageFilterPython.py",
line 1308, in binary_morphological_opening_image_filter
    instance = itk.BinaryMorphologicalOpeningImageFilter.New(*args, **kwargs)
File "{my_path}\.conda\envs\itkex\lib\site-packages\itk\support\template_class.py", line 733, in New
    raise itk.TemplateTypeError(self, input_type)
itk.support.extras.TemplateTypeError: itk.BinaryMorphologicalOpeningImageFilter is not wrapped for input type `None`.
To limit the size of the package, only a limited number of
types are available in ITK Python. To print the supported
types, run the following command in your python environment:

    itk.BinaryMorphologicalOpeningImageFilter.GetTypes()

Possible solutions:
(...)

    e.g.: instance = itk.BinaryMorphologicalOpeningImageFilter[itk.Image[itk.SS,2], itk.Image[itk.SS,2], 
itk.FlatStructuringElement[2]].New(my_input)

(...)

Supported input types:

itk.Image[itk.SS,2]
itk.Image[itk.UC,2]
itk.Image[itk.US,2]
itk.Image[itk.F,2]
itk.Image[itk.D,2]
itk.Image[itk.SS,3]
itk.Image[itk.UC,3]
itk.Image[itk.US,3]
itk.Image[itk.F,3]
itk.Image[itk.D,3]
itk.Image[itk.SS,4]
itk.Image[itk.UC,4]
itk.Image[itk.US,4]
itk.Image[itk.F,4]
itk.Image[itk.D,4]

I had already noticed thatitk.BinaryMorphologicalOpeningImageFilter is not wrapped for input type None did not seem right, but when I inspect the image type (or do a print(watershed_image.__class__)) I get the expected image type at the output of the WatershedImageFilter, i.e. itk.itkImagePython.itkImageULL3, having np.uint64 data type.

I had also tried casting the watershed image prior to applying the binary morphological operation, e.g.

cast_img = itk.cast_image_filter(watershed_image, ttype=(watershed_image.__class__, uchar_image_type))

But despite what you point @dzenanz in the *.wrap file, unfortunately Python suggests that there is a problem with the input image (so similar to passing None):

  File "/ITKEx/src/Segmentation/Watersheds/SegmentWithWatershedAndDistanceMap/Code.py", line 130, in <module>
    cast_img = itk.cast_image_filter(watershed_image, ttype=(watershed_image.__class__, uchar_image_type))
  File "{my_path}\.conda\envs\itkex\lib\site-packages\itk\support\helpers.py", line 173, in image_filter_wrapper
    return image_filter(*args, **kwargs)
  File "{my_path}\.conda\envs\itkex\lib\site-packages\itk\itkCastImageFilterPython.py", line 24150,
 in cast_image_filter
    instance = itk.CastImageFilter.New(*args, **kwargs)
  File "{my_path}\.conda\envs\itkex\lib\site-packages\itk\support\template_class.py", line 723, in New
    raise RuntimeError(

RuntimeError: No suitable template parameter can be found.

Please specify an input via the first argument, the 'ttype' keyword parameter,
or via one of the following keyword arguments: Input, InputImage, Input1

The binary morphological filter does work (have checked that applying to another image upstream in the pipeline with the right types works), so the output of the watershed seems to be the issue.

So I run out of ideas.

jhlegarreta avatar Sep 08 '21 00:09 jhlegarreta

Doing

segmented_clean_image = itk.binary_morphological_opening_image_filter(
  watershed.GetOutput(), kernel=structuring_element)

in the SegmentWithWatershedImageFilter example, right here, results in the same error pointed in https://github.com/InsightSoftwareConsortium/ITKSphinxExamples/pull/86#issuecomment-914718199.

So I'd dare to say that the output of the watershed image filter (?) does not have the right type information under any circumstance (i.e. unrelated to the exampled in this PR). Or else, the binary opening image filter does not correctly infer the type, since the watershed image does have the expected type when being inspected at the IDE/printing its type. Does this make sense?

Additionally, having read again Kevin's example, I am not sure whether the binary opening on the watershed would by itself be equivalent to what the original examples seeks to do. So easiest might be to end the Code.py file by writing the watersheds output, and defer to the analysis script to perform the opening as Kevin's notebook but building a new image on each iteration from the data array to feed ITK's opening filter.

jhlegarreta avatar Sep 11 '21 21:09 jhlegarreta

end the Code.py file by writing the watersheds output

This is preferable to keeping this PR open indefinitely. But before that, I will give it a crack, maybe even tomorrow.

dzenanz avatar Sep 13 '21 21:09 dzenanz

The Python test now passes. I also fixed a C++ compile error when legacy is OFF. Can you clean up this contribution so we can get it merged?

@jhlegarreta please squash my commit into yours (attribution not necessary). I made it a separate commit so you can clearly see what the fix was.

dzenanz avatar Sep 15 '21 21:09 dzenanz

@dzenanz Thanks :100: . Will do that when I get some time. I will need to check the outputs and see if we finally get the whole thing working as in Kevin's notebook !

jhlegarreta avatar Sep 15 '21 22:09 jhlegarreta