Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

morphologie operator clean up #385

Open
StRigaud opened this issue Oct 29, 2024 · 2 comments · May be fixed by #386
Open

morphologie operator clean up #385

StRigaud opened this issue Oct 29, 2024 · 2 comments · May be fixed by #386

Comments

@StRigaud
Copy link
Member

tl:dr: morphological operation with inconsistent name or algorithm, and we try to rename them to make more sense and scalable

We have a bit of inconsistancy around morphology operation, between grayscale and binary, and how to use them.

From the prototype (same for sphere pattern):

  • dilate_box, erode_box -> binary operation each with their kernel, no radius parameter
  • minimum_box, maximum_box -> grayscale operation (equivalent to dilate and erode but grayscale), with radius parameter
  • opening_box, closing_box -> grayscale operation relying on the tier1 kernel minimum_box and maximum_box, , with radius parameter

In CLIc, same as the prototype with the following renaming:

  • dilate_box and erode_box -> dilate and erode
  • minimum_box, maximum_box -> minimum_filter and maximum_filter
  • opening_box, closing_box -> opening and closing

This leads now to 2 morphological processing pipeline available:

  • grayscale with minimum and maximum filter reused in opening and closing, box shaped only, with radius parameter
  • binary with erode and dilate but nearly not reused in other filters, and no radius parameter

To this we have to add user request for custom structuring element operations which should run on grayscale and binary. I belive it would be nice to be able to specify any structuring element.

Here is the proposed reorganisation of name with legacy for the prototype


I will try to aim for the following structure:

binary filter with pre-encoded structuring element :

  • binary_dilate, binary_erode -> encapsulate dilate_box, erode_box (and sphere)
  • binary_opening, binary_closing -> reuse binary_dilate, binary_erode (and sphere)
  • legacy with prototype: dilate_box, erode_box (and sphere)

grayscale filter with pre-encoded structuring element :

  • grayscale_dilate, grayscale_erode -> encapsulate alias for minimum and maximum filter (box only, no sphere)
  • grayscale_opening, grayscale_closing -> reuse grayscale_dilate, grayscale_erode (box only, no sphere)
  • legacy with prototype: minimum_box, maximum_box, opening_box, closing_box (box only, no sphere)

custom structuring element grayscale (also work on binary):

  • erosion and dilation (take a custom strel parameter)
  • opening and closing -> reusing erosion and dilation (take a custom strel parameter)

Side question:

  • strel element behing rename footprint ?

👋 @haesleinhuepf a quick opinion on this? deprecation of some filters can be done later.

@StRigaud StRigaud linked a pull request Oct 29, 2024 that will close this issue
@haesleinhuepf
Copy link
Member

Hey @StRigaud,

in general, having a footprint parameter is a good idea and I agree, we should make names more consistent. Just a remark regarding the footprints: I experimented with this during clij1 times and concluded that a second image parameter (footprint) slows down the operations substantially. That's why current implemenations use for loops with math inside instead of footprints. Hence, keeping versions of the functions without footprint might be good for performance.

Cheers,
Robert

@StRigaud
Copy link
Member Author

keeping versions of the functions without footprint might be good for performance

Yes, not planning to remove them. At best add missing operations like opening and close for binary.

Now keeping 'box' and 'sphere' in function name make lots of sense.

Deprecation in future will only concerns naming.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants