-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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 the lu_solve function #21981
Added the lu_solve function #21981
Conversation
Thanks for contributing to Ivy! 😊👏 |
@zaeemansari70 @edaehn @thecoder12 @ivy-leaves Please I need to you review this PR now. This is because I should be in the next internship hiring stage by now. I am already 1 week close to my starting date and my PR has not been accepted yet. Please review this PR and tell me changes to effect immediately you are reading this! |
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.
Just requested some changes,
Thanks!
|
||
@to_ivy_arrays_and_back | ||
def lu_solve(lower_upper, perm, rhs, validate_args=False, name=None): | ||
return ivy.solve(lower_upper, perm, rhs, validate_args=False, name=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.
ivy.solve()
takes on 2 arguments, also I'd suggest you should read the docstring
on how does this function work.
I'd suggest you implement this compositionally for now. If you don't know what compositionally means I'd refer you to our deep dive in the docs.
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 can't find any information like this in the deep dive doc, please. Can you help?
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.
@zaeemansari70 Please check what I just did!
test_flags=test_flags, | ||
fn_tree=fn_tree, | ||
on_device=on_device, | ||
input=input[0], |
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.
It is not expecting input
as an argument according to the function definintion.
You have to handle lower_upper
, perm
, rhs
, validate_args
for it to work
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.
Requested some changes. 🙂
|
||
@to_ivy_arrays_and_back | ||
def lu_solve(lower_upper, rhs): # ToDO | ||
return ivy.solve(lower_upper, rhs) |
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.
Here you go, this might help you 🙂 https://unify.ai/docs/ivy/docs/functional/ivy/linear_algebra/ivy.functional.ivy.linear_algebra.solve.html#solve
|
||
|
||
@to_ivy_arrays_and_back | ||
def lu_solve(lower_upper, rhs): # ToDO |
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 function definition should be all the arguments present here https://www.tensorflow.org/api_docs/python/tf/linalg/lu_solve
If you are working on an open task, please edit the PR description to link to the issue you've created. For more information, please check ToDo List Issues Guide. Thank you 🤗 |
Closing due to inactivity |
Close #21980