Skip to content

TST add summary call to keras 1d and 2d tests. #888

Merged
changhongw merged 8 commits intokymatio:devfrom
MuawizChaudhary:keras_add_summary_test
Jun 20, 2022
Merged

TST add summary call to keras 1d and 2d tests. #888
changhongw merged 8 commits intokymatio:devfrom
MuawizChaudhary:keras_add_summary_test

Conversation

@MuawizChaudhary
Copy link
Copy Markdown
Collaborator

@MuawizChaudhary MuawizChaudhary commented Jun 16, 2022

This PR tests if we actually fix an error claimed to be fixed by #883 #882 which resolve issues #775 #801.

@MuawizChaudhary MuawizChaudhary mentioned this pull request Jun 16, 2022
Merged
@MuawizChaudhary
Copy link
Copy Markdown
Collaborator Author

MuawizChaudhary commented Jun 16, 2022

We dont got a keras 3d frontend fyi.

lostanlen
lostanlen approved these changes Jun 16, 2022
View reviewed changes
Comment thread tests/scattering1d/test_keras_scattering1d.py Outdated
Comment thread tests/scattering2d/test_keras_scattering2d.py
lostanlen
lostanlen requested changes Jun 16, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@lostanlen lostanlen left a comment

Choose a reason for hiding this comment

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

need some assert's

@MuawizChaudhary
Copy link
Copy Markdown
Collaborator Author

MuawizChaudhary commented Jun 16, 2022

need some assert's

I'm not well versed in regexes, got an example of how I could extract what I want from the printed output?

@MuawizChaudhary
Copy link
Copy Markdown
Collaborator Author

MuawizChaudhary commented Jun 16, 2022

NVM i've figured it out.

janden
janden requested changes Jun 16, 2022
View reviewed changes
Comment thread tests/scattering1d/test_keras_scattering1d.py
Comment thread tests/scattering1d/test_keras_scattering1d.py
@MuawizChaudhary MuawizChaudhary requested a review from janden June 16, 2022 16:54
lostanlen
lostanlen approved these changes Jun 18, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@lostanlen lostanlen left a comment

Choose a reason for hiding this comment

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

thanks

Comment thread tests/scattering1d/test_keras_scattering1d.py
@lostanlen
Copy link
Copy Markdown
Collaborator

lostanlen commented Jun 18, 2022

i have attempted a rebase from #854

@lostanlen
Copy link
Copy Markdown
Collaborator

lostanlen commented Jun 19, 2022

i think that the test failure stems from loading Q = data['Q'] as a float, not int.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Jun 19, 2022

Codecov Report

Merging #888 (7f6bb67) into dev (e89ec79) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev     #888   +/-   ##
=======================================
  Coverage   87.94%   87.94%           
=======================================
  Files          64       64           
  Lines        2257     2257           
=======================================
  Hits         1985     1985           
  Misses        272      272
Flag Coverage Δ
jenkins_main 87.94% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out 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 e89ec79...7f6bb67. Read the comment docs.

@lostanlen lostanlen requested a review from cyrusasfa June 19, 2022 15:24
janden
janden approved these changes Jun 20, 2022
View reviewed changes
Copy link
Copy Markdown
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.

Ok.

@lostanlen
Copy link
Copy Markdown
Collaborator

lostanlen commented Jun 20, 2022

@cyrusvahidi @changhongw can one of you merge this? thanks!

@lostanlen lostanlen requested a review from changhongw June 20, 2022 12:25
@changhongw changhongw merged commit d761ec5 into kymatio:dev Jun 20, 2022
eickenberg pushed a commit that referenced this pull request Jul 5, 2022
@MuawizChaudhary @lostanlen @eickenberg
TST add summary call to keras 1d and 2d tests. (#888)
b41d213 
* TST adding model.summary() to keras 1d tests. Fails without rebasing to dev.

* TST adding model.summary() to keras 2d tests. Fails without rebasing to #883

* STY add space

* STY indentation

* TST now assert

* TST now assert

* fix Keras tests

Co-authored-by: Vincent Lostanlen <vincent.lostanlen@ls2n.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lostanlen lostanlen lostanlen approved these changes

@janden janden janden approved these changes

@cyrusasfa cyrusasfa Awaiting requested review from cyrusasfa

@changhongw changhongw Awaiting requested review from changhongw

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@MuawizChaudhary @lostanlen @codecov-commenter @janden @changhongw