boostorg / math

Boost.org math module
http://boost.org/libs/math
Boost Software License 1.0
313 stars 226 forks source link

Correct spurious underflow issues in non-central beta and t (also eff… #1134

Closed jzmaddock closed 6 months ago

jzmaddock commented 6 months ago

…ects nc-F via beta).

See https://github.com/scipy/scipy/issues/20693.

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 93.69%. Comparing base (a53b013) to head (92f5e2b).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/boostorg/math/pull/1134/graphs/tree.svg?width=650&height=150&src=pr&token=26IDb1OAa7&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg)](https://app.codecov.io/gh/boostorg/math/pull/1134?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg) ```diff @@ Coverage Diff @@ ## develop #1134 +/- ## =========================================== - Coverage 93.71% 93.69% -0.03% =========================================== Files 771 771 Lines 61105 61156 +51 =========================================== + Hits 57264 57299 +35 - Misses 3841 3857 +16 ``` | [Files](https://app.codecov.io/gh/boostorg/math/pull/1134?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg) | Coverage Δ | | |---|---|---| | [...lude/boost/math/distributions/non\_central\_beta.hpp](https://app.codecov.io/gh/boostorg/math/pull/1134?src=pr&el=tree&filepath=include%2Fboost%2Fmath%2Fdistributions%2Fnon_central_beta.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg#diff-aW5jbHVkZS9ib29zdC9tYXRoL2Rpc3RyaWJ1dGlvbnMvbm9uX2NlbnRyYWxfYmV0YS5ocHA=) | `91.79% <100.00%> (+0.08%)` | :arrow_up: | | [include/boost/math/distributions/non\_central\_t.hpp](https://app.codecov.io/gh/boostorg/math/pull/1134?src=pr&el=tree&filepath=include%2Fboost%2Fmath%2Fdistributions%2Fnon_central_t.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg#diff-aW5jbHVkZS9ib29zdC9tYXRoL2Rpc3RyaWJ1dGlvbnMvbm9uX2NlbnRyYWxfdC5ocHA=) | `97.64% <100.00%> (-0.12%)` | :arrow_down: | | [test/test\_nc\_t.hpp](https://app.codecov.io/gh/boostorg/math/pull/1134?src=pr&el=tree&filepath=test%2Ftest_nc_t.hpp&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg#diff-dGVzdC90ZXN0X25jX3QuaHBw) | `99.02% <100.00%> (+0.04%)` | :arrow_up: | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/boostorg/math/pull/1134/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg) ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/boostorg/math/pull/1134?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/boostorg/math/pull/1134?dropdown=coverage&src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg). Last update [a53b013...92f5e2b](https://app.codecov.io/gh/boostorg/math/pull/1134?dropdown=coverage&src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boostorg).
jzmaddock commented 6 months ago

@mborland can you see why the codecov report is failing? All the changed lines seem to be covered.

mborland commented 6 months ago

@mborland can you see why the codecov report is failing? All the changed lines seem to be covered.

It looks like there were some large indirect changes: https://app.codecov.io/gh/boostorg/math/pull/1134/indirect-changes/. Did your diffs change the regions expm1 are tested in? Doesn't look like it to me. If they all look benign to you merging will re-baseline the total project coverage.

jzmaddock commented 6 months ago

It looks like there were some large indirect changes: https://app.codecov.io/gh/boostorg/math/pull/1134/indirect-changes/. Did your diffs change the regions expm1 are tested in? Doesn't look like it to me. If they all look benign to you merging will re-baseline the total project coverage.

I don't see how, and I certainly didn't effect the barycentic_rational code... merging.