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

single precision support in skimage.metrics module #5220

Merged
merged 13 commits into from Jul 15, 2021

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Feb 4, 2021

Description

#5219 should be reviewed first since this PR is built on top of that one (to reuse the same _float_dtype utility).

Computations of means as in mse were intentionally set to use dtype=float64 and that is preserved here to keep good accuracy on larger images. Similarly, for structural_similarity, I added single precision support in most of the function, but kept the final mean computation to generate mssim in double precision.

Checklist

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.

@grlee77 grlee77 added the ⏩ type: Enhancement Improve existing features label Feb 4, 2021
@pep8speaks
Copy link

pep8speaks commented Feb 4, 2021

Hello @grlee77! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-07-15 08:58:04 UTC

add float32 support for structural similarity

Metrics give float64 scalar outputs.
Array outputs preserve the precision of the input.
Copy link
Member

@rfezzani rfezzani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just left some minor comments, otherwise, everything is fine 😉 thank you @grlee77.

@@ -226,8 +227,8 @@ def structural_similarity(im1, im2,
# to avoid edge effects will ignore filter radius strip around edges
pad = (win_size - 1) // 2

# compute (weighted) mean of ssim
mssim = crop(S, pad).mean()
# compute (weighted) mean of ssim. Use float64 for accuracy.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

skimage/metrics/tests/test_structural_similarity.py Outdated Show resolved Hide resolved
@grlee77 grlee77 added this to the 0.19 milestone Jul 8, 2021
Copy link
Member

@mkcor mkcor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @grlee77: I'm only this far (tiny gesture) from approving/merging!

skimage/metrics/simple_metrics.py Outdated Show resolved Hide resolved
skimage/metrics/_structural_similarity.py Outdated Show resolved Hide resolved
skimage/metrics/tests/test_simple_metrics.py Outdated Show resolved Hide resolved
skimage/metrics/tests/test_simple_metrics.py Outdated Show resolved Hide resolved
grlee77 and others added 2 commits July 15, 2021 00:13
Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
set dtype properly in test_nmi_random

Co-authored-by: Marianne Corvellec <marianne.corvellec@ens-lyon.org>
@mkcor
Copy link
Member

mkcor commented Jul 15, 2021

Weird... at b94bce2 test_PSNR_float[float16] is failing because of line 69 in skimage/metrics/tests/test_simple_metrics.py; I fetched your branch and saw the same thing locally; to have tests pass, I had to do:

diff --git a/skimage/metrics/tests/test_simple_metrics.py b/skimage/metrics/tests/test_simple_metrics.py
index a69b9abb3..edcb28fd4 100644
--- a/skimage/metrics/tests/test_simple_metrics.py
+++ b/skimage/metrics/tests/test_simple_metrics.py
@@ -66,7 +66,7 @@ def test_PSNR_float(dtype):
     with expected_warnings(['Inputs have mismatched dtype']):
         p_mixed = peak_signal_noise_ratio(cam / 255.,
                                           np.float32(cam_noisy / 255.))
-    assert_almost_equal(p_mixed, p_float64, decimal=5)
+    assert_almost_equal(p_mixed, p_float64, decimal=decimal)

But here on GitHub line 69 reads assert_almost_equal(p_mixed, p_float64, decimal=decimal) 🤔
Rerunning all CI workflows just in case... Have you done any crazy rebasing? 😅

@mkcor
Copy link
Member

mkcor commented Jul 15, 2021

Although GitHub would display otherwise, the code base definitely had assert_almost_equal(p_mixed, p_float64, decimal=5), apparently:
decimal
That's why I took the liberty of committing 508415e, which passed the CI tests.

@mkcor mkcor merged commit 64364a6 into scikit-image:main Jul 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⏩ type: Enhancement Improve existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants