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

Test 1d #623

Merged
merged 31 commits into from Jul 22, 2020
Merged

Test 1d #623

merged 31 commits into from Jul 22, 2020

Conversation

MuawizChaudhary
Copy link
Collaborator

This PR adds a whole suite of tests for the Torch, Tensorflow, and Numpy backend primitives. Additionally, tests for scattering and tests for backend primitives are broken into two separate files. Perhaps we want to do something similar with frontend tests?

Additionally, some already existing backend functions have been modified to perform type checking, such as adding a complex check to the subsampling functions in the numpy and tensorflow backends.

@MuawizChaudhary
Copy link
Collaborator Author

we should really finish up the R2C pr first tho.

@codecov-commenter
Copy link

codecov-commenter commented Jun 28, 2020

Codecov Report

Merging #623 into dev will decrease coverage by 9.30%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev     #623      +/-   ##
==========================================
- Coverage   88.35%   79.05%   -9.31%     
==========================================
  Files          64       64              
  Lines        2319     2320       +1     
==========================================
- Hits         2049     1834     -215     
- Misses        270      486     +216     
Flag Coverage Δ
#jenkins_main ?
#travis 79.05% <100.00%> (+0.26%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
kymatio/scattering1d/backend/numpy_backend.py 100.00% <100.00%> (+2.50%) ⬆️
kymatio/scattering1d/backend/tensorflow_backend.py 100.00% <100.00%> (+2.77%) ⬆️
kymatio/backend/torch_skcuda_backend.py 0.00% <0.00%> (-100.00%) ⬇️
...matio/scattering1d/backend/torch_skcuda_backend.py 0.00% <0.00%> (-100.00%) ⬇️
...matio/scattering2d/backend/torch_skcuda_backend.py 0.00% <0.00%> (-100.00%) ⬇️
...matio/scattering3d/backend/torch_skcuda_backend.py 0.00% <0.00%> (-85.19%) ⬇️
kymatio/backend/torch_backend.py 93.24% <0.00%> (-6.76%) ⬇️
kymatio/scattering3d/utils.py 87.87% <0.00%> (ø)
kymatio/scattering2d/filter_bank.py 100.00% <0.00%> (ø)
kymatio/scattering3d/filter_bank.py 94.91% <0.00%> (ø)
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb14b0b...d51eac3. Read the comment docs.

kymatio/scattering1d/backend/numpy_backend.py Show resolved Hide resolved
tests/scattering1d/test_numpy_backend_1d.py Outdated Show resolved Hide resolved
tests/scattering1d/test_numpy_backend_1d.py Show resolved Hide resolved
tests/scattering1d/test_numpy_backend_1d.py Show resolved Hide resolved
tests/scattering1d/test_numpy_backend_1d.py Show resolved Hide resolved
tests/scattering1d/test_numpy_backend_1d.py Outdated Show resolved Hide resolved
tests/scattering1d/test_tensorflow_backend_1d.py Outdated Show resolved Hide resolved
tests/scattering1d/test_tensorflow_backend_1d.py Outdated Show resolved Hide resolved
@MuawizChaudhary
Copy link
Collaborator Author

are we ready for merge?

assert 'should be complex' in record.value.args[0]

def test_fft():
x = torch.randn(2, 1)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Remove.

@janden janden self-requested a review July 2, 2020 18:14
Copy link
Collaborator

@janden janden left a comment

Choose a reason for hiding this comment

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

Just add that blank line and it's ready for merge.

@MuawizChaudhary
Copy link
Collaborator Author

Added!

@eickenberg
Copy link
Collaborator

yup seems good to go to me

@janden
Copy link
Collaborator

janden commented Jul 17, 2020

Let's make sure the tests pass, then we're ready to merge.

@MuawizChaudhary
Copy link
Collaborator Author

The tests pass on my end. I can try changing the tolerance and see if that changes things

@MuawizChaudhary
Copy link
Collaborator Author

Looks like jenkins passes.

@janden
Copy link
Collaborator

janden commented Jul 20, 2020

Still failing on Travis. Can you increase the tolerance for that test? Also make sure to put a comment explaining why we need the increased tolerances.

@janden janden merged commit 4de0f31 into kymatio:dev Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants