Fix benchmarking CI job

[hakkelt]

When trying to fix the failing benchmarking CI job, I found AirspeedVelocity.jl. I tried running the benchmarks with it, and I found it more intuitive, both when running benchmarks locally from the CLI and when writing the configuration for the GitHub action. Additionally, it prints the benchmarking results into a comment. This package also removes the need for the custom benchmark-running script runbenchmarks.jl.

[hakkelt]

Accidentally, I added a commit to this PR that I intended to add to my other PR, so I revoked it with a forced push.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.86%. Comparing base (383d978) to head (e96d204).

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #154      +/-   ##
==========================================
- Coverage   94.91%   90.86%   -4.05%     
==========================================
  Files          78       77       -1     
  Lines        2477     2376     -101     
==========================================
- Hits         2351     2159     -192     
- Misses        126      217      +91     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[hakkelt]

I reviewed the logs of the 3 failed actions, and here are my findings:

• Test / Julia 1.9 - macOS-latest - x64: I cannot really understand this one. The error messages:

  valid: Test Failed at /Users/runner/work/ProximalOperators.jl/ProximalOperators.jl/test/runtests.jl:55
    Expression: all(isapprox.(y_prealloc, y, rtol = rtol, atol = 100 * eps(R)))
  valid: Test Failed at /Users/runner/work/ProximalOperators.jl/ProximalOperators.jl/test/runtests.jl:56
    Expression: all(isapprox.(y_naive, y, rtol = rtol, atol = 100 * eps(R)))
  

  • The stacktrace is not really helpful. These checks are in the prox_test function (runtests.jl:38), and they are called from test_indPolyhedral.jl, in the first case of the testset named "valid" that checks IndPolyhedral(l, A).
  • This error only appears on macOS, and it appeared after I merged master into this PR. Strangely, this test passed on master...
  • @lostella: could you re-trigger this test because my guess is that it's only a transient error. But the fact that the seed of Random is fixed in test_calls.jl, which gets included before test_indPolyhedral.jl, so if it's a transient error, it is probably not a random-generator-related problem.
• codecov/project: My theory is that the coverage decreased because the Julia 1.9 + macOS test job failed, possibly because this Julia release inlined fewer functions. I checked the first couple of files in the codecov report, and the lines that lost coverage in these files are the following:

 is_convex(f::Type{<:IndBallL2}) = true
 is_set_indicator(f::Type{<:IndBallL2}) = true
 is_convex(f::Type{<:IndHyperslab}) = true
 is_set_indicator(f::Type{<:IndHyperslab}) = true
 is_convex(f::Type{<:SqrHingeLoss}) = true
 is_set_indicator(f::Type{<:IndBinary}) = true
 is_convex(f::Type{<:NuclearNorm}) = true

• Benchmark: The benchmarking script completes, but it cannot publish the results as a comment in this PR because 'Resource not accessible by integration'. It seems like another permission problem. The documentation doesn't mention any changes to repo settings, and I use it in one of my projects; it worked without any extra configuration. I guess it will also work after getting merged to master, or maybe if you re-trigger this action.

I also opened this PR on my fork, and all actions completed successfully. Here is a sample of the output of AirspeedVelocity.jl in this comment from the PR on my fork: hakkelt#3 (comment)

Anyway, @lostella, what do you think about this package/solution? I could not find out what the output of the previous solution looked like because the very old GitHub Actions runs were deleted, and all the more recent runs failed.

[lostella]

@hakkelt AirspeedVelocity is interesting, after a quick look. I will check out this branch locally and try it to get a feeling for how it works. But assuming that it's fine, I would be ok with the change. What's important is that benchmarks run correctly in GH workflows in some way, and that regressions are spotted.

I restarted the 1.9 macOS tests, which appear to have succeeded now. I guess one thing we should consider (maybe in a separate PR) is to bump Julia versions to 1 + LTS in the test workflow? I believe the codecov stuff we could remove. (Also this, separately)

[lostella]

The benchmark report comment looks great btw

[hakkelt]

Yes, I agree that replacing Julia 1.9 in tests with LTS is a good idea. It's pretty unlikely that there are any Julia 1.9 users who want to use the latest ProximalOperators version, but who knows? 😄 On the other hand, there is also a slight chance that tests pass with Julia 1.9, but fail with 1.10... It's possible to test on 1.9, LTS, and the latest, but I think it's not worth wasting electricity on that, and it's probably better to focus on LTS than support outdated Julia versions.

I'm not sure what you mean by removing codecov stuff. Btw, I checked the report on the CodeCov site and realized that only a single coverage report was uploaded for this PR, whereas the last merged PR had 6 reports uploaded, corresponding to the 6 test jobs. I didn't find any warnings or notices anywhere that tell why the reports of the otherwise successful test weren't uploaded. So, I think it is just a spurious regression on code coverage statistics that hopefully gets fixed automatically for the next PR.