Skip to content

Conversation

ckormanyos
Copy link
Member

@ckormanyos ckormanyos commented Jul 3, 2025

This PR obtains more code coverage. But it is for later in 1.90 (not for today's 1.89).

@ckormanyos ckormanyos marked this pull request as draft July 3, 2025 14:23
Copy link

codecov bot commented Jul 3, 2025

Codecov Report

❌ Patch coverage is 99.89259% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 96.2%. Comparing base (55bf069) to head (5b53322).
⚠️ Report is 65 commits behind head on develop.

Files with missing lines Patch % Lines
include/boost/multiprecision/cpp_dec_float.hpp 99.7% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop    #711     +/-   ##
=========================================
+ Coverage     94.7%   96.2%   +1.5%     
=========================================
  Files          296     297      +1     
  Lines        30881   31177    +296     
=========================================
+ Hits         29233   29967    +734     
+ Misses        1648    1210    -438     
Files with missing lines Coverage Δ
example/exercise_threading_log_agm.cpp 100.0% <100.0%> (ø)
include/boost/multiprecision/complex_adaptor.hpp 92.3% <100.0%> (+2.8%) ⬆️
include/boost/multiprecision/cpp_bin_float.hpp 93.9% <ø> (+2.7%) ⬆️
include/boost/multiprecision/cpp_double_fp.hpp 100.0% <100.0%> (ø)
...nclude/boost/multiprecision/detail/default_ops.hpp 93.5% <ø> (+2.5%) ⬆️
...oost/multiprecision/detail/functions/constants.hpp 99.2% <100.0%> (ø)
...lude/boost/multiprecision/detail/functions/pow.hpp 99.4% <100.0%> (+4.7%) ⬆️
...ude/boost/multiprecision/detail/functions/trig.hpp 97.5% <ø> (+0.8%) ⬆️
test/git_issue_167.cpp 100.0% <100.0%> (+50.0%) ⬆️
test/git_issue_464.cpp 100.0% <100.0%> (ø)
... and 17 more

... and 4 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55bf069...5b53322. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ckormanyos
Copy link
Member Author

Ok so I worked on coverage, mostly with focus on cpp_dec_float. That file is pretty well covered now, but missing 8 interesting lines. I'll let this one sit for a while.

I would like to improve the line coverage of the tests and then we can discuss which headers need more dedicated edge tests.

Let's just leave this as a draft until the action on 1.90 wraps up.

@ckormanyos
Copy link
Member Author

ckormanyos commented Jul 5, 2025

In cpp_dec_float going to nearly $99\%$ coverage was straightforward. Sure it needed a few hundred lines of dedicated test code.

I only found 1 bug, but harmless, in the area of negative infinity ($-\infty$). There were several areas having redundant or unclear lines. I cleared up (or subjectively think I cleared up) a bunch of these sequences. Nothing really critical was found, and this shows how well this thing was tested to begin with. I'm really impressed with the test-suite.

@ckormanyos ckormanyos requested a review from jzmaddock August 15, 2025 16:27
@ckormanyos
Copy link
Member Author

ckormanyos commented Aug 16, 2025

OK this one is green again and ready for review.

I'm pretty satisfied with these results. Project coverage is at a solid $96.2\%$, with cpp_double_fp_backend.hpp fully covered and cpp_dec_float missing fewer than $5$ lines.

A few bug fixes were done along the way. In future work, I'd like to see how to hit some of the missing lines in cpp_int.hpp and cpp_bin_float.hpp. I took a few tries at this but only had modest success.

When stressing cpp_bin_float, I ran across #726, which seems like an actual (still non-solved) bug.

There is also a hefty TODO list (for myself) at #703. So after concluding this more_coverage PR, I'll move on to #703 and possibly #726.

Cc: @jzmaddock and @mborland

@jzmaddock
Copy link
Collaborator

Thanks Chris for this, looks good other than my few comments on the header changes.

@jzmaddock
Copy link
Collaborator

When erf(cpp_bin_float(-0.0)) is called, the generic implementation does not catch the negative zero. It simply goes to the Taylor series and returns zero.

Good catch!

My gut says something like:

if(fabs(z) < tools::root_epsilon<T>())
   return invert ? 1 : 2 * z / constants::root_pi<T>();

Which should make that zone more efficient as well as fixing the signed zero issue? Note: just typed in off the top of my head!!

@ckormanyos
Copy link
Member Author

ckormanyos commented Aug 31, 2025

Removed the constexpr-ness efforts, as these will be handled in a future, separate branch. The most modern, mostly passing commit having the bulk of the constexpr-ness efforts is ac1b954.

@ckormanyos ckormanyos merged commit fc508e8 into develop Aug 31, 2025
69 checks passed
@ckormanyos ckormanyos deleted the more_coverage branch August 31, 2025 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants