-
Notifications
You must be signed in to change notification settings - Fork 28
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
Conjugate the first argument in vecdot #201
Conversation
Note that PyTorch uses its own vecdot directly for complex inputs,, which already does conjugation. This is currently untested by the test suite (data-apis/array-api-tests#312) Fixes data-apis#200.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no suggestions.
Comments skipped due to low confidence (1)
array_api_compat/common/_aliases.py:482
- The new behavior of conjugating the first argument in vecdot should be covered by tests.
res = xp.conj(x1_[..., None, :]) @ x2_[..., None]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spec stance on rejecting real arguments to complex functions is a bit bizzare. I realize it's probably because xp.imag(real_array)
would otherwise have to allocate an array of zeros but still.
FWIW, I agree with you that real arguments really should be accepted in At any rate, this isn't a problem here. The reason is that the code in This is something to be aware of generally. In a lot of the wrappers, we use specific implementation details either of |
And also you don't really ever have to worry about array-api-strict in array-api-compat. The only place it's ever used is it can be returned from |
I'm going to merge this now, but I'm hopeful we can get testing for this in array-api-tests before we do a release. |
Note that PyTorch uses its own vecdot directly for complex inputs,, which already does conjugation.
This is currently untested by the test suite (data-apis/array-api-tests#312)
Fixes #200.