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

Added array_equal function and tests #87

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

markkraay
Copy link
Collaborator

While working on #37, noticed that we were using tp.allclose in contexts where we should probably be using something similar to np.array_equal instead. This MR will:

  1. Add tp.array_equal
  2. Correct the test cases with improper usage of tp.allclose (we should probably only be using tp.allclose when working with float tensors)
  3. Change the test cases which are using the np.array_equal(cp.from_dlpack(_).get() trick to use tp.array_equal

@markkraay markkraay self-assigned this Aug 12, 2024
@markkraay markkraay changed the title added array_equal function and tests Added array_equal function and tests Aug 12, 2024
@markkraay markkraay added the tripy Pull request for the tripy project label Aug 12, 2024
[1, 1],
[1, 1]
])
assert tp.array_equal(a, b)
Copy link
Collaborator

@pranavm-nvidia pranavm-nvidia Aug 12, 2024

Choose a reason for hiding this comment

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

asserts are stripped from docs. Probably want:

Suggested change
assert tp.array_equal(a, b)
# doc: print-locals a b is_equal
is_equal = tp.array_equal(a, b)
assert is_equal

(the comment could go at the top of the example)

@markkraay
Copy link
Collaborator Author

@pranavm-nvidia Should we auto-convert input tensors to tp.Tensor for this function? Then we could avoid wrapping the arguments in tp.Tensor and handle this implicitly.

@pranavm-nvidia
Copy link
Collaborator

@pranavm-nvidia Should we auto-convert input tensors to tp.Tensor for this function? Then we could avoid wrapping the arguments in tp.Tensor and handle this implicitly.

I'm not entirely sure. You could make this argument for pretty much all Tripy APIs. Let's discuss offline.

@markkraay markkraay marked this pull request as ready for review August 29, 2024 03:20
Comment on lines +54 to +55
if a.dtype != b.dtype:
b = cast(b, a.dtype)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we return False here?

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

Successfully merging this pull request may close these issues.

2 participants