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

Add array_equal #3189

Merged
merged 4 commits into from Mar 17, 2020
Merged

Add array_equal #3189

merged 4 commits into from Mar 17, 2020

Conversation

rushabh-v
Copy link
Contributor

closes #3125

asi1024
asi1024 reviewed Mar 11, 2020
View reviewed changes
cupy/logic/comparison.py Outdated Show resolved Hide resolved
cupy/logic/comparison.py Outdated Show resolved Hide resolved
cupy/logic/comparison.py Outdated Show resolved Hide resolved
cupy/logic/comparison.py Outdated Show resolved Hide resolved
tests/cupy_tests/logic_tests/test_comparison.py
a = xp.array([1, 2, 3, 4])
b = xp.array([1, 2, 3])
return xp.allclose(a, b)

Copy link
Member

Choose a reason for hiding this comment

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

Please add tests check

  • broadcasts are not allowed
  • comparison with ndarrays of different dtypes is allowed

Copy link
Member

Choose a reason for hiding this comment

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

Why is allclose called instead of array_equal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I actually copy pasted the tests from allclose for my convenience and mistakenly left that call unchanged. I think I should've checked it..

I'll edit that soon.

@rushabh-v
Copy link
Contributor Author

have made all the changes. And all the tests are passing.
Can you take a look, please? @asi1024

@asi1024
Copy link
Member

asi1024 commented Mar 15, 2020

@rushabh-v Could you check that broadcasts are not allowed?

@rushabh-v
Copy link
Contributor Author

Added that test. Can you review it again, please? @asi1024

@rushabh-v
Copy link
Contributor Author

I've just checked whether Numpy and CuPy results same or not for the broadcast case, because numpy.array_equal also doesn't allow broadcasts.

@rushabh-v rushabh-v requested a review from asi1024 March 16, 2020 14:11
@asi1024
Copy link
Member

asi1024 commented Mar 17, 2020

Jenkins, test this please.

@pfn-ci-bot
Copy link
Collaborator

Successfully created a job for commit 1eae082:

@asi1024 asi1024 added the cat:feature New features/APIs label Mar 17, 2020
@asi1024 asi1024 changed the title implement array_equal() Add array_equal Mar 17, 2020
@asi1024 asi1024 added this to the v8.0.0b1 milestone Mar 17, 2020
@asi1024 asi1024 added the st:test-and-merge (deprecated) Ready to merge after test pass. label Mar 17, 2020
@chainer-ci
Copy link
Member

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

@mergify mergify bot merged commit aa0ef83 into cupy:master Mar 17, 2020
@asi1024
Copy link
Member

asi1024 commented Mar 17, 2020

LGTM. Thanks for your PR!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leofang leofang leofang left review comments

@asi1024 asi1024 Awaiting requested review from asi1024

Assignees

@asi1024 asi1024

Labels
cat:feature New features/APIs st:test-and-merge (deprecated) Ready to merge after test pass.
Projects
None yet
Milestone
v8.0.0b1
Development

Successfully merging this pull request may close these issues.

implementing array_equal() for CuPy
5 participants
@rushabh-v @asi1024 @pfn-ci-bot @chainer-ci @leofang