Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 3 3
Lines 281 303 +22
=========================================
+ Hits 281 303 +22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Tests & coverage passing, ready for review or merge. @oscardssmith I have contributor rights on the repo obv but I think it's a good idea to have a second pair of eyes on code before merging. |
|
@oscardssmith I think this addresses your comments. The main point of the PR is still to make Float32 loggamma available that doesn't upcast to Float64 (and make Float16 upcast to F32 until that gets its own implementation too). But it now does it in a more unified way that tries to avoid redoing the same logic. Further comments welcome. 😄 |
Co-authored-by: Oscar Smith <oscardssmith@gmail.com>
|
Okay, asymptotic and stirling merged. Functions instead of consts. Coverage and tests passing. Also fixed a seed in tests because otherwise debugging can be annoying. |
|
Oh and I bumped version to v1.0.1 |
Co-authored-by: Oscar Smith <oscardssmith@gmail.com>
|
Looks great! will merge once tests are passing. |
Perhaps something more sophisticated can be done but this is just the same methods but without upconverting to Float64, dropping some terms in the series evals and creating the constants as Float32 literals too.
I tried doing this with Float16 too but Float16 has no convenient constructors and was a lot more subtle on what a good quality approximation should return, so I left it for now - though I did change it to upconvert to Float32 instead of Float64.
I also significantly hardened the test cases, now we tests 10k+ cases for real and complex values of Float16, Float32 and Float64, hopefully this will contribute towards preventing accuracy regression in the future.