celerity / celerity-runtime

High-level C++ for Accelerator Clusters
https://celerity.github.io
MIT License
139 stars 18 forks source link

CI: Use Docker image from ghcr.io for benchmarks #248

Closed fknorr closed 6 months ago

fknorr commented 6 months ago

This update was missed when transitioning celerity_ci.yml to pull from ghcr.io.

This also switches us to the pinned version of hipSYCL / ACpp to ensure we do not mistake performance improvements / regressions in the SYCL implementation for ones in Celerity. I've updated the benchmark results to set an appropriate baseline.

github-actions[bot] commented 6 months ago

Check-perf-impact results: (09393feddbe509742e2a3b35b8d3214d)

:warning: Significant slowdown (>1.25x) in some microbenchmark results: 8 individual benchmarks affected
:rocket: Significant speedup (<0.80x) in some microbenchmark results: building command graphs in a dedicated scheduler thread for N nodes - 1 > immediate submission to a scheduler thread / chain topology

Relative execution time per category: (mean of relative medians)

coveralls commented 6 months ago

Pull Request Test Coverage Report for Build 7875514063

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details


Files with Coverage Reduction New Missed Lines %
src/grid.cc 1 97.29%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 7854326363: -0.04%
Covered Lines: 4998
Relevant Lines: 5194

💛 - Coveralls
github-actions[bot] commented 6 months ago

Check-perf-impact results: (4c65f1399a47e0eb1340f63004745b17)

:question: No new benchmark data submitted. :question:
Please re-run the microbenchmarks and include the results if your commit could potentially affect performance.

fknorr commented 6 months ago

The benchmark results above are bogus due to a performance bug in our benchmarking suite - see #250. I have removed the new results from this PR after all.