clEsperanto / pyclesperanto

GPU-accelerated Image Processing library using OpenCL
https://clesperanto.github.io/
BSD 3-Clause "New" or "Revised" License
31 stars 6 forks source link

[Bug] bounding_box() on uint8 data is limited to values < 256 #164

Closed thawn closed 6 months ago

thawn commented 6 months ago

Pre-requist tasks

Describe the bug the results of bounding_box() on a uint8 image are limited to uint8 values. Any value that should be above 255 becomes 255

this is particularly annoying, because threshold_otsu returns uint8 images

To Reproduce


import pyclesperanto as cle
import pyclesperanto_prototype as clep
import numpy as np

#create sample image
image = np.zeros((300, 300), dtype=np.uint8)
image[240:290, 20:260] = 1

#measure the bounding box with pyclesperanto
cle.bounding_box(image) # returns [20.0, 240.0, 240.0, 255.0, 255.0, 1.0]

#measure the bounding box with pyclesperanto_prototype
clep.bounding_box(image) # returns [20.0, 10.0, 0, 259.0, 29.0, 0]

**Expected behavior**
the results of bounding_box() should be independent of the image data type (I guess uint32 should be the default return data type)

**Configuration details:**
 - OS: [e.g. MacOS 12.7.2]
 - Device: [Apple M1 Max]
 - Device type: [Integrated GPU]
thawn commented 6 months ago

Cool thanks, Stephane 🎉

thawn commented 5 months ago

just noticed that the bug is fixed for the x coordinate but still persists for the y and z coordinates.

I guess you need to apply similar changes for y and z as you did for x in this commit

thawn commented 5 months ago

@StRigaud would you prefer to reopen this bug, or should I file a new one?

thawn commented 5 months ago

actually, I suggest to fix the bug in line 17 of tier1::multiply_image_and_position_func():

rather than creating an output array of the same type as the input, that function should create an output that can handle positions larger than 255 (e.g. int64 if the input is an integer and float64 if the input is a float).

With its current behavior, tier1::multiply_image_and_position_func() implicitly assumes that the position has the same type as the input, while it is actually of type uint32 (or even uint64?).

StRigaud commented 5 months ago

uint64 and int64 are not supported in OSx arm64 so I would enforce 32 bits as much as possible for now.