-
Notifications
You must be signed in to change notification settings - Fork 78
Implement B2 index in C #2354
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
Implement B2 index in C #2354
Conversation
ce416b7 to
608f53b
Compare
Codecov Report
@@ Coverage Diff @@
## main #2354 +/- ##
==========================================
- Coverage 93.30% 93.28% -0.02%
==========================================
Files 28 28
Lines 26797 26840 +43
Branches 1233 1229 -4
==========================================
+ Hits 25002 25038 +36
- Misses 1762 1769 +7
Partials 33 33
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
608f53b to
0606a8a
Compare
benjeffery
left a comment
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. I haven't gone into the numerical issues, but happy for this to go in. Shall we add an issue to deal with those separately?
| handle_library_error(err); | ||
| goto out; | ||
| } | ||
| ret = Py_BuildValue("d", result); |
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.
Assuming this coverage error is spurious.
| stack.append((v, path_product)) | ||
| return total_proba | ||
| # Let Python decide if the base is acceptable | ||
| math.log(10, base) |
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.
Ha, nice.
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.
Took me a good while to figure this out!
Closes tskit-dev#2252 Closes tskit-dev#2256
0606a8a to
16166da
Compare
I don't think there's much we can do tbh. We'd spend a week trying to figure out where precision is lost, and almost certainly nobody would ever hit it. |
Serious tedium encountered dealing with logs of a general base in C. I've done my best to get the corner cases covered, but there may be dragons lurking in obscure corners 🤷
Closes #2252
Closes #2256