Fix float32 support in denoise_bilateral and denoise_tv_bregman#3936
Fix float32 support in denoise_bilateral and denoise_tv_bregman#3936alexdesiqueira merged 3 commits intoscikit-image:masterfrom
Conversation
See skimage/_shared/fused_numerics.pxd for the definition of np_floats, which is currently float32 and double.
|
Thanks @jni! |
|
@meeseeksdev backport to v0.14.x |
…al and denoise_tv_bregman
|
@meeseeksdev backport to v0.15.x |
…al and denoise_tv_bregman
| int i = 0 | ||
| np_floats lam = 2 * weight | ||
| np_floats rmse = DBL_MAX | ||
| double rmse = DBL_MAX |
There was a problem hiding this comment.
why was this line changed, the whole point of this was to keep float32 as float32 as long as possible. wasa it causing errors?
There was a problem hiding this comment.
I presume that setting a float32 to DBL_MAX does not do good things. Perhaps I there is a FLT_MAX?
There was a problem hiding this comment.
ah i see. I didn't know what DBL meant, there is a way to switch code paths between dtypes for a fused type. I guess we would need to do that here.
There was a problem hiding this comment.
We could just set it to FLT_MAX regardless of dtype. If your initial RMSE exceeds that, you're in deep trouble 😂
Anyway, the code works, it's just probably a tiny bit slower because of float32-float64 comparisons in the float32 input case.
There was a problem hiding this comment.
yeah, this "optimization" would be a 0.16 branch only IMO.
…6-on-v0.14.x Backport PR #3936 on branch v0.14.x (Fix float32 support in denoise_bilateral and denoise_tv_bregman)
…6-on-v0.15.x Backport PR #3936 on branch v0.15.x (Fix float32 support in denoise_bilateral and denoise_tv_bregman)
Description
Fixes #3449, again.
See #3449 (comment) and subsequent replies for details.
Checklist
Docstrings for all functionsGallery example in./doc/examples(new features only)Benchmark in./benchmarks, if your changes aren't covered by anexisting benchmark
For reviewers
later.
__init__.py.doc/release/release_dev.rst.@meeseeksdev backport to v0.14.x