Skip to content
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

add @benchmarkset & @case for defining benchmark suite #147

Merged
merged 6 commits into from
Apr 5, 2021

Conversation

Roger-luo
Copy link
Contributor

@Roger-luo Roger-luo commented Sep 23, 2019

This PR adds 2 macros to provide a simpler syntax for defining large benchmark suite.

I found it can be a bit annoying if there are multiple benchmark suite like this:

https://github.com/QuantumBFS/YaoArrayRegister.jl/blob/master/benchmark/benchmarks.jl

it is not necessary to manually control things like SUITE["matrices"]["contiguous"], thus I implement these two macro to make defining benchmark suite in a similar syntax as the testset in Test.

now you can just write things like

@suite "suite 1" begin
     @case k for k in 1:n
            rand($k,  $k) * rand($k, $k)
      end
end

instead of

SUITE = BenchmarkGroup()
SUITE["suite 1"] = BenchmarkGroup()

for k in 1:n
    SUITE["suite 1"][k] = @benchmarkable rand($k, $k) * rand($k, $k)
end

PS. currently it will generate a constant SUITE, which is compatible with PkgBenchmark

Update: I find I need some more modification on this new syntax, the current one still fails to deal with some cases.

@Roger-luo Roger-luo changed the title RFC: add @suite & @case for defining benchmark suite [WIP/RFC]: add @suite & @case for defining benchmark suite Sep 24, 2019
@Roger-luo Roger-luo changed the title [WIP/RFC]: add @suite & @case for defining benchmark suite add @benchmarkset & @case for defining benchmark suite Jan 10, 2021
@Roger-luo
Copy link
Contributor Author

I changed the name to @benchmarkset and @case, this will provide a similar way to define benchmark suite to @testset and @test

@vchuravy
Copy link
Member

Can you rebase onto master to get CI?

@codecov-io
Copy link

Codecov Report

Merging #147 (9fb6e11) into master (5df4174) will decrease coverage by 0.23%.
The diff coverage is 81.81%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #147      +/-   ##
==========================================
- Coverage   86.37%   86.14%   -0.24%     
==========================================
  Files           6        6              
  Lines         624      635      +11     
==========================================
+ Hits          539      547       +8     
- Misses         85       88       +3     
Impacted Files Coverage Δ
src/BenchmarkTools.jl 100.00% <ø> (ø)
src/groups.jl 94.67% <81.81%> (-1.53%) ⬇️

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 5df4174...9fb6e11. Read the comment docs.

@Roger-luo
Copy link
Contributor Author

bump just fixed the tests.

@Roger-luo Roger-luo requested a review from vchuravy April 1, 2021 08:28
@vchuravy vchuravy merged commit 598075e into JuliaCI:master Apr 5, 2021
@Roger-luo Roger-luo deleted the group-macro branch April 5, 2021 22:05
@Roger-luo
Copy link
Contributor Author

hi @vchuravy mind releasing a patch version for this?

@KristofferC
Copy link
Contributor

Just pointing out, this package was intentionally designed to not use macros since they were considered confusing and unclear:

JuliaCI/PkgBenchmark.jl#9 (comment)

@Roger-luo
Copy link
Contributor Author

Just pointing out, this package was intentionally designed to not use macros since they were considered confusing and unclear:

@KristofferC Ah that makes sense, I didn't know one can use this package with addgroup! , it seems not being mentioned in manual at all, only in https://github.com/JuliaCI/BenchmarkTools.jl/blob/master/doc/reference.md#misc-functions

I feel need to polish the documentation a bit for this if we want to revert this PR. I'm OK with both, not sure what other people thinks.

@KristofferC
Copy link
Contributor

Well, too late now.

@awadell1
Copy link

Is the equivalent using addgroup! something like:

# Root Suite
SUITE = BenchmarkGroup()

# BenchmarkGroup generated by @benchmarkset
s = BenchmarkGroup()

# For loop for each @case 
for k in 1:n
    s[k] = @benchmarkable rand($k, $k) * rand($k, $k)
end

addgroup!(SUITE, s)

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.

5 participants