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

ENH: add iswtn (n-dimensional inverse SWT) #256

Merged
merged 2 commits into from
Dec 28, 2016
Merged

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Dec 19, 2016

In release 0.5.1 there is swtn for an n-dimensional forward SWT with flexible choice of axes to transform. This PR provides the corresponding inverse operation, iswtn.

The redundancy of the SWT grows quickly as the number of dimensions (and transform levels) increases, but I think it is still worth having this generalized implementation available.

This logic here may be a bit hard to follow with all the slice objects, use of itertools.product for nested loops etc. However, it is a direct extension of the approach used in iswt2 (i.e. combining all possible combinations of odd/even shifts of idwtn along the transformed axes). The tests are pretty thorough, so I am fairly confident that the implementation is correct.

This makes iswt2 a bit redundant, but I think keeping a simpler 2D example around serves as a useful reference in understanding the approach behind iswtn.

@codecov-io
Copy link

codecov-io commented Dec 19, 2016

Current coverage is 86.96% (diff: 100%)

Merging #256 into master will increase coverage by 0.23%

@@             master       #256   diff @@
==========================================
  Files            20         20          
  Lines          2910       2962    +52   
  Methods           0          0          
  Messages          0          0          
  Branches        280        297    +17   
==========================================
+ Hits           2524       2576    +52   
  Misses          335        335          
  Partials         51         51          

Powered by Codecov. Last update eca5bac...1ce334a

@rgommers
Copy link
Member

Also from the perspective of symmetry with other functions (idwt2/idwtn) I think it's fine to have both functions.

Needs a rebase.

@rgommers rgommers added this to the v1.0 milestone Dec 27, 2016
@@ -167,6 +169,7 @@ def test_swt2_ndim_error():
assert_raises(ValueError, pywt.swt2, x, 'haar', level=1)


@dec.slow
Copy link
Member

Choose a reason for hiding this comment

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

Can we have one test not marked as slow for iswt2 and iswtn? The default for test() is not to run slow tests, so easy to be developing for some time without running any tests for these functions otherwise.

These do not require a slow decorator, but only test a single wavelet type.
@grlee77
Copy link
Contributor Author

grlee77 commented Dec 27, 2016

This has now been rebased. I added versions of the 2D and nD tests that just call the existing tests for a single wavelet type so that they are very fast.

Most of the other changes in the tests file are minor simplifications: use the kind='discrete' keyword when generating the lists of wavelets to test to avoid the need for type checking later.

@rgommers
Copy link
Member

Test failures are unrelated to this PR, due to the new coverage version released yesterday. I'll have a look at fixing that.

@rgommers
Copy link
Member

@grlee77 need to rebase again or manually trigger the builds again after merging gh-259.

@rgommers
Copy link
Member

Allright, all green after restarting the build now that coverage 4.3.1 is on PyPI.

@rgommers rgommers merged commit 9b63a71 into PyWavelets:master Dec 28, 2016
@rgommers
Copy link
Member

Merged, thanks @grlee77!

@grlee77 grlee77 deleted the iswtn branch December 30, 2016 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants