-
Notifications
You must be signed in to change notification settings - Fork 17
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
Improve SquaredL2SquaredAbsLoss
#278
Conversation
Codecov Report
@@ Coverage Diff @@
## main #278 +/- ##
=======================================
Coverage 93.85% 93.86%
=======================================
Files 51 51
Lines 3696 3701 +5
=======================================
+ Hits 3469 3474 +5
Misses 227 227
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
scico/test/functional/test_loss.py
Outdated
|
||
def test_cubic_root(): | ||
N = 10000 | ||
p, _ = uniform(shape=(N,), dtype=snp.float32, minval=-10.0, maxval=10.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.
consider seeding rng here so the test result is not stochastic
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.
Fixed.
scico/test/functional/test_loss.py
Outdated
def test_cubic_root(): | ||
N = 10000 | ||
p, _ = uniform(shape=(N,), dtype=snp.float32, minval=-10.0, maxval=10.0) | ||
q, _ = uniform(shape=(N,), dtype=snp.float32, minval=-10.0, maxval=10.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.
jax gotcha: I think p and q will be equal if you make them this way. Need to get the key from the first call and pass it to the second.
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.
Indeed they were. Fixed.
Improve algorithm, docs, and testing of the cubic equation root calculation used by
SquaredL2SquaredAbsLoss
.