PyWavelets / pywt

PyWavelets - Wavelet Transforms in Python
http://pywavelets.readthedocs.org
MIT License
2.04k stars 470 forks source link

idwtn doesn't tolerate unequal shapes in coefficients #54

Closed mrzv closed 8 years ago

mrzv commented 10 years ago

The following two snippets of code should be equivalent, I think:

import numpy as np
import pywt

a = np.random.randn(64,64)
w = pywt.Wavelet('bior3.3')
#w = pywt.Wavelet('db2')
#w = pywt.Wavelet('db1')

coeffs  = pywt.dwtn(a,w)
coeffs2 = pywt.dwtn(coeffs['aa'], w)
coeffs['aa'] = pywt.idwtn(coeffs2, w)
print coeffs['aa'].shape, coeffs['dd'].shape
#b1 = pywt.idwtn(coeffs, w)

coeffs = pywt.dwt2(a,w)
coeffs2 = pywt.dwt2(coeffs[0], w)
coeffs = pywt.idwt2(coeffs2, w), coeffs[1]
print coeffs[0].shape, coeffs[1][0].shape
b2 = pywt.idwt2(coeffs, w)

#print np.max(np.fabs(b1-b2))

However, if one uses 'db2' or 'bior3.3' wavelet (the only one that I know works is 'db1'), the first version crashes in the commented line (that generates b1), because not all coefficients have the same shape.

rgommers commented 9 years ago

@mrzv thanks for the bug report. I can reproduce this. Getting (36, 36) (35, 35) for bior3.3 and (34, 34) (33, 33) for db2. Results in:

ValueError                                Traceback (most recent call last)
<ipython-input-7-ac20e1df9d72> in <module>()
     11 coeffs['aa'] = pywt.idwtn(coeffs2, w)
     12 print coeffs['aa'].shape, coeffs['dd'].shape
---> 13 b1 = pywt.idwtn(coeffs, w)
     14 
     15 

/home/rgommers/Code/tmp/pywt/pywt/multidim.pyc in idwtn(coeffs, wavelet, mode, take)
    293         raise ValueError("`coeffs` must contain at least one non-null wavelet band")
    294     if any(s != coeff_shape for s in coeff_shapes):
--> 295         raise ValueError("`coeffs` must all be of equal size (or None)")
    296 
    297     if take is not None:

ValueError: `coeffs` must all be of equal size (or None)

This needs investigating.

kwohlfahrt commented 9 years ago

Just a few thoughts:

If the input to the rounding functions is not an exact integer, then information is lost about the size of the input data. idwt2 uses the correct_size option to idwt to work around this, which allows the size of the a and d coefficients to differ by at most one and as far as I can tell discards the extra pixel in the larger input. I have not looked into the details of convolution.c so I'm unsure about the actual behaviour.

I wasn't sure how to handle this when I first wrote the idwtn function, so I left it alone with the assumption that it would only be used for a single-level transform. I'm happy to work on fixing the issue, but would like some advice on where to start - there are a lot of variations in convolution.c with e.g. double_downsampling_convolution, double_downsampling_convolution_full, double_downsampling_convolution_valid_sf and I'm not sure how they all interact.

kwohlfahrt commented 9 years ago

Moving the discussion here from #67. This is a policy/interface question rather than an implementation question, so I think I'll leave it up to @rgommers.

I would be in favour of keeping the strict behaviour of idwtn, and in fact extending this to idwt2 and idwt after the API break, as the multilevel functions will be available for this purpose.

rgommers commented 9 years ago

@kwohlfahrt I haven't looked at this in a lot of detail, but my first impression is that I agree with you - better to be a bit strict rather than silently truncating which could lead to hard to find bugs in user code.

kwohlfahrt commented 8 years ago

I think this can be closed (not a bug, a feature :P ). Will do so in a couple days if there are no objections.

grlee77 commented 8 years ago

+1 from me on closing this