JuliaLinearAlgebra / Octavian.jl

Multi-threaded BLAS-like library that provides pure Julia matrix multiplication
https://julialinearalgebra.github.io/Octavian.jl/stable/
Other
226 stars 18 forks source link

Try CI codecov on nightly too, to see if codecov is faster #142

Closed IanButterworth closed 1 year ago

IanButterworth commented 2 years ago

Based on https://github.com/JuliaPerf/STREAMBenchmark.jl/pull/15#issuecomment-958749496 Octavian seems to be a good test for the new path-specific code coverage tracking that's on julia nightly now (https://github.com/JuliaLang/julia/pull/44359).

Just a draft PR to see how times compare

cc. @chriselrod

codecov[bot] commented 2 years ago

Codecov Report

Merging #142 (ca9dfda) into master (db713f3) will decrease coverage by 3.54%. The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #142      +/-   ##
==========================================
- Coverage   88.72%   85.18%   -3.55%     
==========================================
  Files          13       13              
  Lines         878      938      +60     
==========================================
+ Hits          779      799      +20     
- Misses         99      139      +40     
Impacted Files Coverage Δ
src/forward_diff.jl 83.01% <0.00%> (-14.76%) :arrow_down:
src/complex_matmul.jl 87.36% <0.00%> (-12.64%) :arrow_down:
src/memory_buffer.jl 68.00% <0.00%> (-5.92%) :arrow_down:
src/macrokernels.jl 84.33% <0.00%> (-4.86%) :arrow_down:
src/funcptrs.jl 97.33% <0.00%> (-2.67%) :arrow_down:
src/matmul.jl 90.98% <0.00%> (-1.85%) :arrow_down:
src/block_sizes.jl 85.88% <0.00%> (-1.03%) :arrow_down:
src/utils.jl 67.10% <0.00%> (-0.96%) :arrow_down:
src/global_constants.jl 70.00% <0.00%> (+0.61%) :arrow_up:

Continue to review full report at Codecov.

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

chriselrod commented 2 years ago
 coverage=true/Julia nightly/3 threads/ubuntu-latest/x64/pull_request
succeeded 2 minutes ago in 6m 50s 

Yeah, that's way faster.

I'd be in favor of disabling coverage on CI except for nightly, honestly.

IanButterworth commented 2 years ago

Great.

I noticed after submitting this that there's another CI config file for nightly. This PR might be duplicating runs?

chriselrod commented 2 years ago

I noticed after submitting this that there's another CI config file for nightly. This PR might be duplicating runs?

The CI situation seems a little weird. Seems nightly doesn't split tests by eltype, but Julia release does.

Seems nightly wasn't tested with coverage at all before, while Julia "1" has coverage and no-coverage tests.

EDIT: There's no duplication because you only added nightly to the Julia coverage tests, and nightly was only run without coverage previously.

IanButterworth commented 2 years ago

By the way, the new codecov option will be on 1.8 but needs manual override, which needs exposing in julia-actions/julia-runtest for that to work.

Maybe this PR is evidence enough that Pkg.test should switch to this in 1.8.. given the 2nd beta hasn't gone out yet, that may be a reasonable thing to do, but I already got this in after the feature freeze, so I'm not sure.

chriselrod commented 2 years ago

By the way, the new codecov option will be on 1.8 but needs manual override, which needs exposing in julia-actions/julia-runtest for that to work.

Does this mean we need some change somewhere?

Maybe this PR is evidence enough that Pkg.test should switch to this in 1.8.. given the 2nd beta hasn't gone out yet, that may be a reasonable thing to do, but I already got this in after the feature freeze, so I'm not sure.

I would definitely be in favor. Note that with coverage=true, every single Julia nightly test has finished but none of the Julia 1 tests have.

(One Julia 1 test with coverage=false has finished; coverage=false runs more tests).

It'll probably be a long time before the first coverage=true test with Julia 1 actually finishes.

chriselrod commented 2 years ago

With coverage enabled, Julia 1 takes anywhere from half an hour to over 4 hours: https://github.com/JuliaLinearAlgebra/Octavian.jl/runs/5476242675?check_suite_focus=true

chriselrod commented 2 years ago

It is odd, however, that codecov is reporting <50% coverage.

IanButterworth commented 2 years ago

Does this mean we need some change somewhere?

Currently 1.8 would need something like this, but the Pkg.test arg julia_args isn't exposed in julia-runtest yet

      - uses: julia-actions/julia-runtest@v1
        with:
          julia_args: `--code-coverage=@`

It is odd, however, that codecov is reporting <50% coverage.

Yeah, I'll open a PR testing nightly only and see what that reports

IanButterworth commented 2 years ago

I suspect the codecov different is a real bug. I saw some small changes in Base when writing tests for it, so I'm not surprised. Although the reason isn't obvious.

I'm looking into it