8000 Simplify `F.deconvolution_2d` test by ishanrai05 · Pull Request #6498 · chainer/chainer · GitHub
[go: up one dir, main page]
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.

Sign up for GitHub

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

Jump to bottom

Simplify F.deconvolution_2d test #6498

Merged
merged 3 commits into from
Apr 19, 2019
Merged

Simplify F.deconvolution_2d test #6498

merged 3 commits into from
Apr 19, 2019

Conversation

ishanrai05
Copy link
Contributor

Simplifies test_deconvolution_2d using testing.FunctionTestCase.

Solves part of issue #6071

@hvy hvy added the cat:test Test or CI related. label Mar 13, 2019
@niboshi niboshi mentioned this pull request Mar 13, 2019
Closed
@ishanrai05
Copy link
Contributor Author

@Crissman Can you please review this?

@Crissman
Copy link
Member
Crissman commented Apr 2, 2019

Jenkins, test this please.

@chainer-ci
Copy link
Member

Jenkins CI test (for commit cf441af, target branch master) failed with status FAILURE.
(For contributors, please wait until the reviewer confirms the details of the error.)

@Crissman
Copy link
Member

Jenkins shows 10+ errors on the Deconvolution_2Dd tests, so we need to confirm whether relevant or not.

@ishanrai05
Copy link
Contributor Author

@Crissman I think the tolerances need to be adjusted again.

@ishanrai05
Copy link
Contributor Author

@Crissman I am unable to reproduce the errors on my system.

@Crissman
Copy link
Member

Ok. Let's retest with Jenkins. Jenkins, test this please.

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit cf441af (21936d4):

@chainer-ci
Copy link
Member

Jenkins CI test (for commit cf441af, target branch master) failed with status FAILURE.

@Crissman
Copy link
Member

Jenkins still showing Deconvolution2D results off:
x: array(0.07680343490716937)
y: array(0.07691694685886355)

Need to look to see the cause of discrepancy.

@ishanrai05
Simplify F.deconvolution_2d test
488eedb 
Simplifies `test_deconvolution_2d` using `testing.FunctionTestCase`.
@ishanrai05
Copy link
Contributor Author
ishanrai05 commented Apr 16, 2019 •
edited
Loading

@Crissman I have added the tolerances for backward_test for float64 and float32 dtype since the tests that were failing were of these data types.
Also all the tests failing were on python2. Maybe that is why I was unable to produce the errors on my system.
Can you take a look again? :)

@Crissman
Copy link
Member

Jenkins, test this please.

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 57da88e (7f1d7f8):

@chainer-ci
Copy link
Member

Jenkins CI test (for commit 57da88e, target branch master) failed with status FAILURE.

@Crissman
Copy link
Member

Merging.

@Crissman Crissman merged commit 99efa4a into chainer:master Apr 19, 2019
@Crissman
Copy link
Member

Oops. Merged before clear on the error tolerances. Unmerging. 😓

@Crissman Crissman mentioned this pull request Apr 19, 2019
Merged
@imos
Copy link
Member
imos commented Apr 19, 2019

FYI, I found some flakiness in test_deconvolution_2d and test_deconvolution_nd tests:
https://ci.preferred.jp/chainer.py37.gpu/3525/
https://ci.preferred.jp/chainer.py37.gpu/3517/

@kmaehashi kmaehashi added this to the v7.0.0a1 milestone May 16, 2019
@ishanrai05 ishanrai05 deleted the test-22 branch October 11, 2019 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Crissman Crissman

Labels
cat:test Test or CI related.
Projects
None yet
Milestone
v7.0.0a1
Development

Successfully merging this pull request may close these issues.
7 participants
@ishanrai05 @Crissman @chainer-ci @pfn-ci-bot @imos @kmaehashi @hvy
0