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

bugfix: missing detections of minima/maxima at image border #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

haesleinhuepf
Copy link
Member

Hey Stephane @StRigaud ,

this closes #23 . This was tested in CLIJ2 manually and in pyclesperanto_prototype via built-in tests. See

I presume, you already fixed it in the clesperanto/clic branch? If not, I'm happy to send another PR. just let me know.

Feel free to comment here. Otherwise, I'll merge in a week or so.

Best,
Robert

@StRigaud
Copy link
Member

StRigaud commented Aug 8, 2022

For what I have tested, this is not enough. You need to also skip the central pixel position otherwise you will always detect a local maxima on border with no maxima around:

input      output
0 0 0      1 0 0
0 0 0  ->  0 0 0
0 0 0      0 0 0

more here :

To make it clean, I now save the pixel position needed

const int4 centerPos = (int4){x,y,z,0};            // central pixel position saved before loop
int4 localPos = localMaxPos + (int4){rx,ry,rz,0};  // local pixel position being evaluated in the loop

and I use the OpenCL integrated function any and all to test if vectors are equals as such:

if( all(localPos >= 0) && any(localPos != centerPos) )

I only fixed the detect_maxima.cl so far but the rest is in the to do list.

@StRigaud
Copy link
Member

StRigaud commented Aug 8, 2022

Also, not sure relevant for your side for now, but you may want to add a cast to float when initializing localMin/Max value

float localMax = READ_src_IMAGE(src, sampler, pos).x - 1;

into

float localMax = (float) READ_src_IMAGE(src, sampler, pos).x - 1;

otherwise it will fail for unsigned type inputs as the -1 will apply before the float conversion.

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.

Detect Maxima/Minima border effect bug
2 participants