add n-dimensional support to denoise_wavelet #2242
Conversation
also make multichannel default to False to match other denoising routines
Current coverage is 90.68% (diff: 100%)@@ master #2242 diff @@
==========================================
Files 303 303
Lines 21763 21783 +20
Methods 0 0
Messages 0 0
Branches 2012 2012
==========================================
+ Hits 19731 19755 +24
+ Misses 1669 1667 -2
+ Partials 363 361 -2
|
|
👍 for this approach, and LGTM. This would be how to handle, e.g., several MRI sequences which are inherently independent. For color, though, we clearly need something a little different. Working on each channel in isolation actually can exacerbate color noise. That's an issue for a different PR, though (discuss in #2240). |
| @@ -394,7 +394,8 @@ def _wavelet_threshold(img, wavelet, threshold=None, sigma=None, mode='soft'): | |||
| return pywt.waverecn(denoised_coeffs, wavelet) | |||
|
|
|||
|
|
|||
| def denoise_wavelet(img, sigma=None, wavelet='db1', mode='soft'): | |||
| def denoise_wavelet(img, sigma=None, wavelet='db1', mode='soft', | |||
| multichannel=False): | |||
There was a problem hiding this comment.
In the docstring, please, change expected data type for img to smth like (M[, N[, ..., P]][, C]), and add description for multichannel parameter.
|
I plan to update the tests to add a 4D test case. |
| @@ -400,7 +400,7 @@ def denoise_wavelet(img, sigma=None, wavelet='db1', mode='soft', | |||
|
|
|||
| Parameters | |||
| ---------- | |||
| img : ndarray (2D/3D) of ints, uints or floats | |||
| img : ndarray ([M[, N[, ...P]][, C]) of ints, uints or floats | |||
There was a problem hiding this comment.
Sorry, actually this should be (M, N[, ...P][, C]) if we declare 2+D support. Please, notice that [, ] mark optional dimensions.
There was a problem hiding this comment.
I think 1D is already supported here, though?
edit: 1D through 4D operation now verified via new tests
There was a problem hiding this comment.
True. Let it be (M[, N[, ...P]][, C]) then.
|
One last remark, otherwise LGTM. |
| multichannel=multichannel) | ||
| res2 = restoration.denoise_wavelet(noisy, sigma=0.1, | ||
| multichannel=multichannel) | ||
| assert (res1.sum()**2 <= res2.sum()**2) |
There was a problem hiding this comment.
I know this wasn't your approach, but this test seems quite weak to me! How about showing a reduction in the noise after denoise_wavelet, or increase in SNR?
There was a problem hiding this comment.
sure. it should be simple to verify at least a rough improvement via compare_psnr
|
@grlee77 thank you! This PR makes me very happy. What do you think of my test suggestion? We do have a policy that new PRs shouldn't have to fix old code issues, so feel free to ignore. |
|
Tests for 1D through 4D inputs were added and PSNR is compared. PSNR only improves slightly with the default sigma (~10%). To get a reasonably denoised image, the user has to specify sigma larger than the true value. This was resolved in #2241, so I will not plan to address that in this PR. |
|
Brilliant! Thank you, @grlee77! |
Description
This adds n-dimensional support to
denoise_wavelet. The multichannel handling matches the existingdenoise_tv_chambollewhich also has n-dimensional support.