-
Notifications
You must be signed in to change notification settings - Fork 121
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
Automatic benchmarking on pull requests #468
base: master
Are you sure you want to change the base?
Conversation
module YaoArrayRegisterBenchmarks | ||
using ..BenchmarkUtils: replace_imports | ||
using Yao | ||
using Yao.YaoBlocks: sprand_hermitian |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This line can probably be deleted once the benchmark is fixed)
Wow! Thanks for setting this up! Yeah, those benchmarks were made for this exact same feature but because missing of a lot of infrastructure 2 years ago they gradually get abandoned. I'd like to play with this a bit and I have one question actually: how do you decide if the benchmark gets run or not? Or there is a separate option to turn the benchmark off/on in the commit message or comments? I would expect this to drain the GitHub action capacity quickly because the benchmarks in this repo are quite resource-heavy. |
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #468 +/- ##
==========================================
+ Coverage 88.28% 88.88% +0.59%
==========================================
Files 77 83 +6
Lines 4824 4840 +16
==========================================
+ Hits 4259 4302 +43
+ Misses 565 538 -27 ☔ View full report in Codecov by Sentry. |
Right now all benchmarks defined in the
I haven’t tried this but that sounds like a good idea. If you look at |
Maybe https://github.com/marketplace/actions/find-comment could be used to check for any comment on the PR disabling the benchmarks. Not sure if it’s worth it though |
Just thinking out loud, but it might be nice if AirspeedVelocity.jl had a CLI option to filter benchmarks, like: benchpkg Yao -r master,patch-1 -e 'expensive1' -e 'expensive2' -e 'suite/expensive3*' although I'm not sure how to implement this with Comonicon.jl |
This is an attempt to set up automatic benchmarking on every pull request to Yao.jl.
This PR creates a GitHub action that uses AirspeedVelocity.jl (easy benchmarking with a CLI based on Comonicon.jl) to automatically run the benchmarks file on all submitted PRs, and create a table of the performance measurements. This is useful for catching performance regressions.
Your project was a bit tricky to setup because there are multiple libraries in one repo. So, I created a main
benchmark/benchmarks.jl
file thatinclude
's the benchmarks in the lib folders. This works as follows:SUITE
withSUITE["YaoArrayRegister"]
equal to theYaoArrayRegister
benchmark suite, etc.using YaoArrayRegister
withusing Yao.YaoArrayRegister
inside the included benchmarks, so that running the benchmark only requiresYao
to be installed (easier to work withAirspeedVelocity.jl
, as it will measure performance over the revision history of the git repo).However, while it does seem to work at the import step, it seems like the benchmark files themselves are two years old and out-of-date. e.g.,
sprand_hermitian
is no longer exported; there is someinstruct!
call that is no longer defined for the passed types, etc.Please let me know how you'd like to proceed. e.g., perhaps you could update the API in the benchmarks file, and then this automated benchmarking could work? Feel free to push to this PR branch.
Cheers,
Miles
An example GitHub PR comment is shown here: SymbolicML/DynamicExpressions.jl#27 (comment)