ADD: ConnectedComponentsLabeling Box operation

[StRigaud]

Add the connected components labelling box to CLIc and the other operation required to make it run.

The main classes do not rely on .cl files but calls various other kernel operation:

• connected components labelling box + test
• close gaps in label map (test is missing)

To runs those two operation, the following kernels were implemented:

• sum reduction + test
• replace intensity + test
• replace intensities (test is missing)
• block enumerate (test is missing)
• set column (test is missing)
• flag existing intensities (test is missing)

This required a minor modification of some .cl filename requested here

I will see to add missing tests before merge. Feedback and review are welcome. Code isn't the most clean sorry :)

Message for futur me: "wrong" data type used in kernel can lead to unstable results. Further investigation and fixing may be required.

[haesleinhuepf]

Hey Stephane,

wow, great work! CCL is one of the most challenging "basic" operations. As you mentioned the missing tests: One of the tests was also missing on python side. I just added it and took the oportunity to rename the function "flag_existing_intensities" to " flag_existing_labels" because that describes better what it's doing.
https://github.com/clEsperanto/pyclesperanto_prototype/blob/flag_existing_labels/tests/test_flag_existing_labels.py

Would you mind renaming it on CLIc side as well?

Furthermore, if you need help with implementing tests and / or specific kernels, let me know! After implementing 400 of them, I became quite efficient. ;-)

Furthermore, I'm still searching for the student who implements kernels for earning money. Stay tuned!

Cheers,
Robert

[StRigaud]

No issue for the re-naming :).

However, don't If you saw yet but this PR is dependent on a .cl renaming on the CLIj side. Otherwise it will not work (hence the build fail on MacOS).
I am waiting for this and to add the missing test to merge :)

[StRigaud]

Change FlagExistingIntensitiesKernel to FlagExistingLabelsKernel.
Add the corresponding test.

The kernel is still named flag_exisinting_intensities_x.cl in clij_opencl_kernel repo. The PR and CLIc will fail until this is fix :)

[StRigaud]

also set column test is not missing anymore.

still missing:

• replace intensities (test is missing)
• block enumerate (test is missing)

[StRigaud]

All test added.

However, I have some doubt on the blocksize parameter used in BlockEnumerateKernel and in SumReductionKernel.
My blocksize value must be >= to my array size otherwise the tests fails. I wouldn't mind a double check on this.

Final check should be on the gateway method name and parameters order fitting the CLIj2 and pyclesperanto method name and parameters.

[haesleinhuepf]

Hey @StRigaud ,

first of all: Happy New Year!

My blocksize value must be >= to my array size otherwise the tests fails.

That's suspicious. The idea of the blocksize is to be smaller than the array size. It might be my mistake again, I had this error earlier. :-(

This change:
clEsperanto/pyclesperanto_prototype@0fcbe1c

Didn't make it to the clij-opencl-kernels repository:
https://github.com/clEsperanto/clij-opencl-kernels/blob/master/src/main/java/net/haesleinhuepf/clij/kernels/block_enumerate.cl#L15

I urgently need to figure out how to use git submodules and restructure how the kernels go into pyclesperanto. Stay tuned. I will do this asap.

I've left some more comments below.

Thanks for again for your great work! I'm really happy to see CLIc evolving.

[StRigaud]

Add reviews modification to the PR. No issues.

For the blocksize:
I applied the .cl correction. No visible effect.

• in SumReductionKernel -> if blocksize is smaller than array length, the test fail. sums do not correspond.
• in BlockEnumerateKernel -> if blocksize is smaller than array length, the test fail:

blocksize = 4
source = 0, 1, 0, 1, 0, 0, 1, 1, 0, 0, 1, 0
valide = 0, 1, 0, 2, 0, 0, 3, 4, 0, 0, 5, 0
result = 0, 1, 0, 2, 0, 0, 1, 2, 0, 0, 1, 0

Again, if blocksize is high enough, all is good

I need to check the code to be sure, I swith the PR to draft again.

[haesleinhuepf]

Hey @StRigaud .

• in BlockEnumerateKernel -> if blocksize is smaller than array length, the test fail:

In the python version of the block_enumerate test, the sum_reduction is used. See here
versus here.

I attempted to fix this and sent you a PR. I can't test it (Fail to create kernel (CL_INVALID_KERNEL_NAME)) I assume this has something to do with my lack of training when it comes to git submodules...

• in SumReductionKernel -> if blocksize is smaller than array length, the test fail. sums do not correspond.

I checked this test and have a suspicion: The output has fixed size [1,1,1]. However, it's supposed to have size original_width / block_size. I just wrote a test on python side and added documentation to make more clear what this kernel is supposed to do.

I think these issues are related to lack of documentation caused by me. I promise to improve.

Let me know if this helps!

Cheers,
Robert

[haesleinhuepf]

Oh, and one more thing: the SumReductionKernel should have the name SumReductionXKernel as it reduces the image in X dimension. We don't have them yet, but sum_reduction_y and ..._z may make sense at some point.

[StRigaud]

I haven't seen that sum_reduction_x was used in the block_enumerate test! I need to recheck the code for both and their corresponding tests.

Will do the renaming at the same time!

And do not worry about the lack of training, we will fix that soon

[StRigaud]

Thanks for the help, all work fine.
I will try to make a cleaner test later on but all checks out!

[StRigaud]

All checks out! 🥇