search

clij / clij2

GPU-accelerated image processing for everyone
https://clij.github.io/clij2
Other
48 stars 14 forks source link

MeanOfMaskedPixels only works when binary image is 0 and 1 #54

Closed schmiedc closed 2 years ago

schmiedc commented 2 years ago

https://github.com/clij/clij2/blob/a13ebc54863d56c5866e4343844764e5cc63cb0c/src/main/java/net/haesleinhuepf/clij2/plugins/MeanOfMaskedPixels.java#L36

The standard imagej binary is 0 and 255. Which yields a NaN result when used in this function. Although instructions of the function specify "only in pixels which have non-zero values in another binary mask image".

haesleinhuepf commented 2 years ago

Hey Christopher @schmiedc ,

great to see you here! I think we could change this line: https://github.com/clij/clij2/blob/a13ebc54863d56c5866e4343844764e5cc63cb0c/src/main/java/net/haesleinhuepf/clij2/plugins/MeanOfMaskedPixels.java#L40

To something like

notEqualConstant(..., 0)

to make the binary image representing all pixels as True which are not 0. Does that make sense?

Looking forward to hear your thoughts!

Best, Robert

schmiedc commented 2 years ago

Yes :) large 3D data lead you to Clij.

It can work. But would allow also feeding non binary images by accident without failing. In this case would be good to first check if it is binary. Something like ImageJ does: https://github.com/imagej/ImageJ/blob/b85cf9ee905d04b9b4c9ecfddf447e44915ba799/ij/process/ByteProcessor.java#L1274

I guess you want to allow two different binary definitions?

haesleinhuepf commented 2 years ago

In this case would be good to first check if it is binary. Something like ImageJ does:

I'm afraid this would be very wasteful processing time wise.

I guess you want to allow two different binary definitions?

No, we should stick to the definition of binary images that is valid in many programming languages world wide: False: pixel==0, True: pixel!=0 See also FAQ

The above suggested change will ensure that I presume. Do you want to send a PR or shall I implement the change?

Thanks for reporting this bug btw!

schmiedc commented 2 years ago

Sounds good! The definition is clear.

I can do the PR.

Anytime :)

schmiedc commented 2 years ago

@haesleinhuepf

Here you go btw: https://github.com/clij/clij2/pull/55

haesleinhuepf commented 2 years ago

Fixed in #55

Thanks @schmiedc !