Skip to content

Fix testing.shaped_random for shape ()#2870

Merged
toslunar merged 5 commits intocupy:masterfrom
niboshi:shaped-random-shape
Dec 26, 2019
Merged

Fix testing.shaped_random for shape ()#2870
toslunar merged 5 commits intocupy:masterfrom
niboshi:shaped-random-shape

Conversation

@niboshi
Copy link
Copy Markdown
Member

@niboshi niboshi commented Dec 24, 2019

Fixes #2869

@niboshi niboshi added cat:bug Bugs to-be-backported Pull-requests to be backported to stable branch labels Dec 24, 2019
@niboshi niboshi added this to the v8.0.0a1 milestone Dec 24, 2019
@niboshi niboshi assigned niboshi and unassigned niboshi Dec 24, 2019
@niboshi niboshi force-pushed the shaped-random-shape branch from 94a76db to 7d69f53 Compare December 24, 2019 10:57
Copy link
Copy Markdown
Member

@toslunar toslunar left a comment

Choose a reason for hiding this comment

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

Instead of .astype(dtype), how about using xp.asarray(..., dtype)?

@niboshi niboshi force-pushed the shaped-random-shape branch from 7d69f53 to c15d8c6 Compare December 25, 2019 05:34
@niboshi
Copy link
Copy Markdown
Member Author

niboshi commented Dec 25, 2019

Thank you for suggestion.
Force-pushed as c15d8c6.

Copy link
Copy Markdown
Member

@toslunar toslunar left a comment

Choose a reason for hiding this comment

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

Could you also fix the boolean case for consistency? LGTM except for it.

@toslunar toslunar self-assigned this Dec 25, 2019
@niboshi niboshi force-pushed the shaped-random-shape branch from 6078e6c to 73d720e Compare December 25, 2019 06:54
@toslunar
Copy link
Copy Markdown
Member

Jenkins, test this please.

@pfn-ci-bot
Copy link
Copy Markdown
Collaborator

Successfully created a job for commit 73d720e:

@chainer-ci
Copy link
Copy Markdown
Member

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

@niboshi niboshi force-pushed the shaped-random-shape branch 2 times, most recently from fe9de14 to e431326 Compare December 25, 2019 08:53
@niboshi
Copy link
Copy Markdown
Member Author

niboshi commented Dec 25, 2019

PTAL

- Parameterize shape
- Add check for array type
- Skip imag check for 0-size arrays (due to a bug)
# Test ranks

# TODO(niboshi): Fix xfail
@pytest.mark.xfail(strict=True, reason='Explicit error types required')
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

strict=True requires further care. In fact, XPASS with numpy 1.12. (AxisError is since 1.13. xfail by a different reason with numpy 1.9.)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

OK, removed in c7f558e.

Copy link
Copy Markdown
Member

@toslunar toslunar left a comment

Choose a reason for hiding this comment

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

LGTM

if 0 not in self.shape:
# TODO(niboshi): imag causes an error if size=0. Fix it.
self.assertTrue(self.xp.all(0 <= a.imag))
self.assertTrue(self.xp.all(a.imag < 10))
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why does 0 <= a.imag < 10 fail?
if 0 not in self.shape: self.assertTrue(self.xp.any(a.imag)) looks correct (without TODO).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is how the test was originally written and I'm not going to change it.
a.imag causes assertion error (#2885).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@toslunar
Copy link
Copy Markdown
Member

Jenkins, test this please.

@pfn-ci-bot
Copy link
Copy Markdown
Collaborator

Successfully created a job for commit c7f558e:

@chainer-ci
Copy link
Copy Markdown
Member

Jenkins CI test (for commit c7f558e, target branch master) succeeded!

@toslunar toslunar merged commit cb37a99 into cupy:master Dec 26, 2019
@niboshi niboshi deleted the shaped-random-shape branch December 26, 2019 05:11
toslunar added a commit to toslunar/cupy that referenced this pull request Dec 26, 2019
Fix `testing.shaped_random` for shape `()`
@emcastillo
Copy link
Copy Markdown
Member

emcastillo commented Dec 26, 2019

v6 backport? (needed for #2891)

niboshi added a commit to niboshi/cupy that referenced this pull request Dec 26, 2019
The bug has been fixed (cupy#2870)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cat:bug Bugs to-be-backported Pull-requests to be backported to stable branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

testing.shaped_random does not accept shape ()

5 participants