scikit-image / scikit-image

Image processing in Python
https://scikit-image.org
Other
6.06k stars 2.22k forks source link

Code audit: usage of `img_as_float` #3373

Open jni opened 6 years ago

jni commented 6 years ago

There's been lots of discussion about our automatic range conversions, especially when it comes to img_as_float. (See for example #3009 and #3052.) In #3052 @stefanv suggested that we do an audit to check which functions will be affected if we don't ensure that floats are in the range [0, 1]. I went through every non-testing use of img_as_float in our code base to check. Below, I've divided the functions into three cases: (1) function will not work correctly if image is out of range; (2) function will return a correspondingly-scaled image (ie it will work correctly, but the output is not scale invariant), and (3) functions where the output will be unchanged (ie we can safely remove range conversion altogether).

Range essential for correctness

Corresponding magnitude change in output

Behaviour unchanged / totally insensitive to image magnitude

Other notes

The above comes from me examining the code. A more robust audit would use @emmanuelle's API crawl to test images on each function, then test 2x image and check how the output changes:

Of course, we would still miss any functions requiring more than one input.

scottstanie commented 6 years ago

Is there any plan for an intermediate fix to allow the functions under "Behaviour unchanged / totally insensitive to image magnitude" to use float32 unscaled images?

for instance with feature.blob_*, right now it's necessary to do something like image.astype('float64'), which makes the img_as_float as no-op and avoids the ValueError.

jni commented 6 years ago

@scottstanie if I'm not mistaken, #3052 would allow this, is this correct? I'm hoping to get that merged later this week... :crossed_fingers:

scottstanie commented 6 years ago

Yep! hadn't seen that but it would fix it exactly as I'd hoped (making the float32 same as float64)

emmanuelle commented 6 years ago

@jni random walker was written to be insensitive to image magnitude, the beta parameter is normalized by img.std() but I just saw that this is a bug, beta should be normalized by the variance and not the standard deviation. Anyway, I think random walker falls in the category of algorithms (like SLIC) where you have to tune a parameter to get what you want.

grlee77 commented 2 years ago

Summarizing from above

Below, I used rescale_to_float to refer to the function currently named img_as_float (see #6318).

Parameter Scaling

For the majority of functions currently using rescale_to_float, removing this just results in an output range that is different by exactly the former rescaling factor. For example, gaussian filtering a uint8 image without rescaling by 1/255.0 will result in an output that is 255 times larger than previously. The functions in skimage.transform and skimage.filters tend to fall in this category.

There are a few exceptions to this behavior to consider:

Visualization functions are one case where we should continue to normalize to [0, 1]. These include feature.plot_matches, feature.draw_haar_like_features, feature.draw_multiblock_lbp, future.rag.show_rag, segmentation.mark_boundaries, that involve visualization with Matplotlib's imshow. Most or all of these currently rescale int inputs, but not float ones, so that should be fixed!

Parameter Scaling

The following table lists functions that have internal use of rescale_to_float, but where appropriate choice of one of the keyword-arguments depends on the absolute image scale. The table below shows the relative scaling that needs to be applied to a parameter when an image is scaled by image = a * image.

function parameter name(s) scaling
feature.blob_log threshold a
feature.blob_dog threshold a
feature.blob_doh threshold a
feature.corner_fast threshold a
feature.CENSURE non_max_threshold a
feature.ORB fast_threshold a
feature.SIFT c_dog?, c_edge? a
restoration.denoise_tv_bregman weight 1 / a
restoration.denoise_tv_chambolle weight a
restoration.denoise_bilateral sigma_color a
restoration.denoise_wavelet sigma (but default sigma=None estimation is scale insensitive) a
segmentation.felzenszwalb sigma a
segmentation.random_walker beta, tol? a?
util.random_noise var a**2

Notes:

alexdesiqueira commented 2 years ago

Thank you for #6318 , @grlee77; it looks great already! I was thinking, though... wouldn't `rescaletogive a different idea on the image context? Couldn't we useconvertto*` instead...? :slightly_smiling_face:

Edit: I asked the same in the past. Any ideas?