Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Builtins frontend #22007
Builtins frontend #22007
Changes from 25 commits
df2fa13
e80aa91
d6cc29d
8c5eca0
e552bff
681cde9
3988d55
320cd61
fe02c23
c840c6b
7ed9f25
85d2534
857e3d8
e5e20ca
be5874f
a272b53
3cd99a8
b4eaff8
e9892e0
ef7545f
2dc4e8b
1135bd5
bf2aab3
d3e8e36
434d8bb
57cfbca
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
If we want lists to get converted, we have to change this to
else
.I think however that ivy functions can already handle array-likes, in which case you would just need to change the comment, not the condition.
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.
Yeah, I just forgot to modify the comment.
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.
Could you add a docstring that explains what this decorator does overall? It is not very obvious when it should be used.
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.
This function should always be used within
@to_ivy_arrays_and_back
decorator to infer the return array type. Let's consider thejax
frontend. In this case, if decorator receivesjax.Array
, its frontend implementation orlist
the output will always be converted to a frontend implementation ofjax.Array
. However, in case ofbuiltins
the behaviour changes to:almost_any_array -> ivy.Array -> almost_any_array
. In other words, we need to track what array to convert back to. So, if our built-in function recievesnp.ndarray
or its frontend implementation, the output will be converted tonp_frontend.ndarray
and so on. It also has a default behaviour. By default we convert the output to a frontend implementation of array which corresponds to current backend. This was done to cover the cases, when function accepts scalar arguments, but returns an array. For example,range
. But yeah I will add a docstring.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 way
inputs_to_ivy_arrays
andoutputs_to_frontend_arrays
have been designed right now they can't be used individually. They would only work when used together into_ivy_arrays_and_back
.E.g.
ret, frontend_array = fn(*args, **kwargs)
would cause an error iffn
was an unwrapped frontend function because there would not be afrontend_array
returned.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.
frontend_array = _infer_return_array(args[0]) if len(args) != 0 else None
there.inputs_to_ivy_arrays
, and then calloutputs_to_frontend_arrays
passingfrontend_array
.frontend_array
as a new argument likeoutputs_to_frontend_arrays(fn: Callable, frontend_array: str)
.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.
Thanks! I'll try this approach!