-
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
feat : (added the leaky relu activation) #27001
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.
PR Compliance Checks
Thank you for your Pull Request! We have run several checks on this pull request in order to make sure it's suitable for merging into this project. The results are listed in the following section.
Conventional Commit PR Title
In order to be considered for merging, the pull request title must match the specification in conventional commits. You can edit the title in order for this check to pass.
Most often, our PR titles are something like one of these:
- docs: correct typo in README
- feat: implement dark mode"
- fix: correct remove button behavior
Linting Errors
- Found type "null", must be one of "feat","fix","docs","style","refactor","perf","test","build","ci","chore","revert"
- No subject found
Thank you for this PR, here is the CI results: This pull request does not result in any additional test failures. Congratulations! |
Hi! @YushaArif99 i have made all the changes and it's passing all the test in local with the maximum 250 examples. here the result. hope now it's ready to merge. 🚀 thank you |
hii @YushaArif99 @KareemMAX still waiting for your review 😞 |
f601264
to
7689445
Compare
hii @YushaArif99 in local i am not getting any failure but in CI it is showing the failed test at paddle backed. even i have tried with max_examples = 250. here the result and i am in latest commit . |
}, | ||
backend_version, | ||
) | ||
def leaky_relu( |
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 also add the inplace update logic to the paddle
backend 🙂
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.
Apologies for the delay in review. Have been a bit swamped with work recently and so haven't had the time to review any PRs 😓
With regard to the PR itself, the implementation looks good overall. I do notice though that you haven't implemented the frontends for the leaky_relu
function. Could you kindly add the frontend implementation as well along with the frontend tests 🙂.
Also, the CI tests for the paddle backend seem to be failing here with the following error:
base = self.function(*self.__args, **self.__kwargs)
/opt/miniconda/envs/multienv/lib/python3.10/site-packages/hypothesis/strategies/_internal/core.py:214: in sampled_from
raise InvalidArgument("Cannot sample from a length-zero sequence.")
E hypothesis.errors.InvalidArgument: Cannot sample from a length-zero sequence.
Could you also kindly look into this 🙂
Hii @YushaArif99 i have made all the changes as you suggested now its passing all the test in my local and CI also. hope not this is ready to merge. 🚀 thanks |
hii @YushaArif99 can you please review PR.bcz today is the last date to merge PR on hacktober fest 😢 |
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.
Hey @samthakur587, thanks for creating the PR. Actually the leaky_relu
function already seems to be present in the api in ivy/functional/ivy/activations.py
😅
Probably we can close this PR @Ishticode, thanks 😄
but i have implemented this for experimental api and i have checked that it is not present there here . can you please check and then i will close this PR if it is already implemented. please try to response soon. 😢 |
Actually adding a function to the main api or the experimental api is the same thing, we basically add new functions to the experimental API and then move those to the main api once they're more stable. Apologies for the delay, but yeah we'd need to close this PR as a result of the function existing in the API already 😅 |
Thanks for explaining. looking forward for more contribution. 🥲 |
added the leaky relu to ivy experimental api
while implementing this the paddle test are not passing. i used the unsupported decorator but still it is taking the int16 as input dtpye. i stuck there . all the backend are passing.
Closes #26096
Checklist