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

implementing array_equal() for CuPy #3125

Closed
portfoliocreator opened this issue Feb 26, 2020 · 3 comments · Fixed by #3189
Closed

implementing array_equal() for CuPy #3125

portfoliocreator opened this issue Feb 26, 2020 · 3 comments · Fixed by #3189
Assignees
Labels

Comments

@portfoliocreator
Copy link

portfoliocreator commented Feb 26, 2020

I was trying to implement array_equal() for Cupy and was thinking along these lines

def array_equal(a1, a2):
    try:
        a1, a2 = cupy.asarray(a1), cupy.asarray(a2)
    except Exception:
        return False
    if a1.shape != a2.shape:
        return False
    return bool(cupy.asarray(a1 == a2).all())

Should I move forward with this ? I would be grateful if someone could guide me.

@emcastillo emcastillo self-assigned this Mar 3, 2020
@emcastillo
Copy link
Member

emcastillo commented Mar 4, 2020

Hello, thanks for being interested in contributing to CuPy!.

1st is that asarray checks are not needed, since cupy functions accepts only cupy arrays. https://docs-cupy.chainer.org/en/stable/reference/difference.html#universal-functions-only-work-with-cupy-array-or-scalar

2nd is that this function currently synchronizes the device when you do return bool(...)
just doing a return (a1==a2).all() should be fine.

The function can be added to the cupy/logic/comparison.py file.

For sending a PR you will need to add a docstring to the function and also add tests for the external behavior.

@rushabh-v
Copy link
Contributor

rushabh-v commented Mar 9, 2020

are you interested in opening a PR? @portfoliocreator
Otherwise, I would be fine doing it!

@portfoliocreator
Copy link
Author

@rushabh-v Yes , sure go ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants