-
Notifications
You must be signed in to change notification settings - Fork 432
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
Feature Addition: PRelu Module #1328
Conversation
3f15bdd
to
7ab4a85
Compare
Adding docs to the book is pending. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1328 /- ##
==========================================
Coverage 78.87% 84.51% 5.64%
==========================================
Files 555 583 28
Lines 62176 64908 2732
==========================================
Hits 49040 54858 5818
Misses 13136 10050 -3086 ☔ View full report in Codecov by Sentry. |
I believe I'm doing the reshape and broadcast wrongly also. Need to check that too. |
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 a quick note. Can we also update the burn-books in these sections with your new activation function and module?
Sure I will add the book section If my implementation is correct |
d670f77
to
798a491
Compare
I think I resolved all the merge conflicts now |
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.
Good job, that's a nice addition, I have some comments, but I don't think they are going to be hard to resolve.
I don't understand how the coverage is decreasing for test function Do I need to add tests for prelu layer in |
@Arjun31415 , Missing |
8590b27
to
e2670b2
Compare
yeah that was silly of me |
fixed bugs in Forward function refactor tests
Added 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.
LGTM from my end. Just take care of Nathaniel's comments.
@nathanielsimard please check. I have made the changes as requested |
Addtion of PRelu Activation Layer
Checklist
run-checks all
script has been executed.Related Issues/PRs
Provide links to relevant issues and dependent PRs.
Changes
Summarize the problem being addressed and your solution.
Testing