-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
frontends.torch.linalg: add support for cross #7656
Conversation
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.
Hi, thanks for submitting a PR. With this task, your goal is to recreate the Torch functions using functions from the Ivy framework. It is possible that you will have to use a combination of Ivy functions in order to recreate this behaviour :)
More info here
@to_ivy_arrays_and_back | ||
@with_unsupported_dtypes({"1.11.0 and below": ("float16",)}, "torch") | ||
def cross(input, other, *, dim=-1, out=None): | ||
return torch_frontend.cross(input, other, dim, out=out) |
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.
You would need to use Ivy functions here, rather than torch functions.
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.
Hi,
Thanks for the review. I have implemented this function according to the instructions provided in the ToDo list here:
Note: If the Linear Algebra function to be implemented has identical behavior to another PyTorch function, you should simply keep an alias in the linalg.py file rather than creating a duplicate implementation.
Here, it is explicitly mentioned not to provide a duplicate implementation of an already implemented PyTorch function that has an identical behavior, in my case that is torch.cross
, but just use it to define an alias in ../torch/linalg.py
as I've done so. The example provided in that ToDo list highlights exactly what I've implemented. Can you kindly cross verify with the guidelines we were provided and what I've implemented? 🙂 Thanks
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.
I have checked and this is the correct protocol :)
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.
Looks good to me, just need you to resolve conflicts and we can go ahead and merge :)
@jkeane508 The conflicts have been resolved :) |
Close #7597