Skip to content

Improve Coverage Part 4 #1262

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

Open
wants to merge 47 commits into
base: develop
Choose a base branch
from
Open

Improve Coverage Part 4 #1262

wants to merge 47 commits into from

Conversation

jzmaddock
Copy link
Collaborator

No description provided.

Copy link

codecov bot commented Apr 28, 2025

Codecov Report

Attention: Patch coverage is 99.70414% with 1 line in your changes missing coverage. Please review.

Project coverage is 95.07%. Comparing base (9f03f29) to head (ef849f3).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
test/sinhc_test.hpp 94.44% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1262      +/-   ##
===========================================
+ Coverage    93.91%   95.07%   +1.16%     
===========================================
  Files          661      792     +131     
  Lines        54866    66879   +12013     
===========================================
+ Hits         51526    63587   +12061     
+ Misses        3340     3292      -48     
Files with missing lines Coverage Δ
include/boost/math/special_functions/airy.hpp 100.00% <100.00%> (ø)
include/boost/math/special_functions/beta.hpp 100.00% <100.00%> (ø)
...st/math/special_functions/detail/ibeta_inverse.hpp 93.26% <100.00%> (ø)
include/boost/math/special_functions/expint.hpp 100.00% <ø> (ø)
...nclude/boost/math/special_functions/fpclassify.hpp 100.00% <ø> (ø)
...e/boost/math/special_functions/jacobi_elliptic.hpp 100.00% <100.00%> (+2.72%) ⬆️
...lude/boost/math/special_functions/jacobi_theta.hpp 100.00% <100.00%> (+5.10%) ⬆️
include/boost/math/special_functions/lambert_w.hpp 100.00% <100.00%> (+4.59%) ⬆️
include/boost/math/special_functions/legendre.hpp 100.00% <100.00%> (+27.72%) ⬆️
include/boost/math/special_functions/log1p.hpp 100.00% <100.00%> (+25.53%) ⬆️
... and 31 more

... and 170 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 9f03f29...ef849f3. 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

Hi John (@jzmaddock) I see you are in lambert_w. I made a note a few days ago that if anyone gets in there, I'd like to see if the inclusions of <typeinfo>, <exception> and <iostream> can be removed. This is one of the few remaining files with those heavyweights in it.

Also <limits> is in twice.

Can they be removed?

@jzmaddock
Copy link
Collaborator Author

Can they be removed?

Looks like it, lets see what happens...

@jzmaddock
Copy link
Collaborator Author

@mborland , @NAThompson , @ckormanyos , another relatively large batch of coverage changes ready to go. As before I'll give it a few days at least before merging in case there are any comments.

@mborland
Copy link
Member

mborland commented May 14, 2025

Maybe for now it would be best to drop the SYCL and CUDA runners? CUDA shows green but if you open the run log there's a bunch of warnings that will be errors at runtime. Once you are done with your bulk changes I can come behind and fix the GPU stuff since, unfortunately, I have to do it out of tree which is a recipe for merge conflicts. SciPy went back to pinning themselves to versions instead of tracking develop, so there's little danger in these things breaking mid release cycle.

Edit: See #1266

@jzmaddock
Copy link
Collaborator Author

Darn. I'll try and take care of the CUDA issues before I merge. Is there a way to make those warnings errors? The CI seems to be rather fragile otherwise, and we shouldn't have to rely on you to constantly fix things up... otherwise the CUDA support will just atrophy and slowly die :(

@jzmaddock
Copy link
Collaborator Author

Would -Werror cross-execution-space-call do it?

@mborland
Copy link
Member

Would -Werror cross-execution-space-call do it?

I can't say I've ever tried that. I can re-engage about getting the GPU runners moved into the Boost CI system. They sit in the CppAlliance account now because of funding, but maybe once this FSC thing is settled the CppAlliance is paying either way?

@jzmaddock
Copy link
Collaborator Author

OK, just a few warnings like these left:

[ 85%] Building CUDA object libs/math/test/CMakeFiles/boost_math-test_ellint_d_float.dir/test_ellint_d_float.cu.o
ptxas warning : Stack size for entry function '_Z9cuda_testPKdS0_Pdi' cannot be statically determined

I don't think those functions have recursions, and certainly I haven't changed them, are these issues?

@jzmaddock
Copy link
Collaborator Author

Ah, take that back, they are recursive... not a new issue though.

@ckormanyos
Copy link
Member

another relatively large batch of coverage changes ready to go. As before I'll give it a few days at least before merging in case there are any comments.

Not a technical comment, but offer up a thanks for doing this. This whole coverage drive in all its parts has been amazing to watch. It is expected to bring Math over the $95\%$ coverage level, which is very excellent!

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.

3 participants