search

ratt-ru / pfb-imaging

Preconditioned forward/backward clean algorithm
MIT License
7 stars 5 forks source link

Daubechies Wavelets #10

Closed sjperkins closed 4 years ago

sjperkins commented 4 years ago

Still a WIP

sjperkins commented 4 years ago

Thanks @landmanbester :duck: :duck: :duck:

landmanbester commented 4 years ago

Oh, nice! Glad to be a good little rubber duck ;-)

sjperkins commented 4 years ago

@landmanbester Could you please review?

sjperkins commented 4 years ago

I've made an effort to reproduce the pywavelets interfaces on the provided functions.

On the reconstruction side of things, pywavelets supports missing approximation/detail coefficients. This PR doesn't support this, but with some type mangling, it may be possible if it's needed. I can only imagine this happening if coefficients got deleted from the dictionary produced by wavedecn call and then fed into a waverecn call.

landmanbester commented 4 years ago

Cool, thanks. I'll have a look asap

landmanbester commented 4 years ago

I've made an effort to reproduce the pywavelets interfaces on the provided functions.

On the reconstruction side of things, pywavelets supports missing approximation/detail coefficients. This PR doesn't support this, but with some type mangling, it may be possible if it's needed. I can only imagine this happening if coefficients got deleted from the dictionary produced by wavedecn call and then fed into a waverecn call.

I don't see this being an issue

sjperkins commented 4 years ago

Thanks @sjperkins this looks good and it seems like it was quite a bit of work. The only thing I would need is to make sure that we get consistent results with varying decomposition levels. We can probably test this by parametrising over level in the waverecn test. I need to follow up on the importance of the mode='periodisation' argument. I'll do some tests once this is merged and also check with Audrey

Parametrising the decomposition tests over level should by easy enough

I suspect we can just use the zeropad mode now. This probably needs a test.

landmanbester commented 4 years ago

Yep, we just need zeropad mode. This should be the simplest so hopefully not too much work to add it in?

sjperkins commented 4 years ago

Yep, we just need zeropad mode. This should be the simplest so hopefully not too much work to add it in?

I think it might be done -- I put the code paths in. I just haven't tested the mode yet.

sjperkins commented 4 years ago

@landmanbester I've parametrised the wavedecn/waverecn tests over

  1. 10 levels
  2. real and complex data
  3. 3 daubechies wavelets
  4. symmetric and zero padding modes.

All the test cases pass. Note that in the case of (1), the number of levels can be too high. pywavelets warns, but in numba this would require dropping to objmode to log the warning. It looks like it may be possible with bleeding edge numba (https://github.com/numba/numba/pull/5674) but I've commented that out for now, since the wavedecn/waverecn compiles take a while and I'd prefer the compilations to be cached.

landmanbester commented 4 years ago

Awesome! I'm gonna merge and test how it parallelises. Thanks @sjperkins for this large and valuable contribution