-
Notifications
You must be signed in to change notification settings - Fork 648
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
Improved Rigor of PReLU
Test
#3498
Improved Rigor of PReLU
Test
#3498
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3498 +/- ##
=======================================
Coverage 52.98% 52.98%
=======================================
Files 94 94
Lines 11206 11206
=======================================
Hits 5937 5937
Misses 5269 5269 ☔ View full report in Codecov by Sentry. |
tests/linen/linen_activation_test.py
Outdated
|
||
from flax import linen as nn | ||
|
||
# Parse absl flags test_srcdir and test_tmpdir. | ||
jax.config.parse_flags_with_absl() | ||
|
||
|
||
class ActivationTest(parameterized.TestCase): | ||
class ActivationTest(JaxTestCase): |
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 think JaxTestCase
is deprecated. Is there a reason why we're using this over parameterized.TestCase
?
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.
Ah, I didn't know about the deprecation and was using it for its array assert methods. I see now the recommended resolution is to use parameterized.TestCase
with the usual numpy testing utils. I'll update accordingly!
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, thanks!
What does this PR do?
Improves the rigor of the existing
PReLU
activation unit test by checking for correctness in produced output, as well as verifying that the correct parameter dictionary is produced upon initializationFixes # (issue)
Checklist
checks if that's the case).
discussion (please add a
link).
documentation guidelines.
(No quality testing = no merge!)
cc: @chiamp