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

Perf folder update #612

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Conversation

matbesancon
Copy link
Contributor

Updating perf folder for Julia 1.x, this will allow updating the results on the readme as well

@matbesancon
Copy link
Contributor Author

To be sure to keep the perf folder up to date, it could be interesting to have a fake mini-benchmark running to be sure the functions are fine

@matbesancon
Copy link
Contributor Author

if someone knows what's going on:

julia> include("perf/benchmark.jl")
=====================================
    Benchmarks for 4×4 matrices
=====================================
StaticArrays compilation time (×3):ERROR: LoadError: UndefVarError: A_mul_B_unrolled not defined
Stacktrace:
 [1] f_unrolled(::Int64, ::SArray{Tuple{4,4},Float64,2,16}) at /home/mbesancon/.julia/dev/StaticArrays/perf/benchmark.jl:19

@coveralls
Copy link

coveralls commented Jun 4, 2019

Coverage Status

Coverage decreased (-0.02%) to 81.697% when pulling 9f82717 on matbesancon:perf into b81044a on JuliaArrays:master.

@c42f
Copy link
Member

c42f commented Jun 5, 2019

UndefVarError: A_mul_B_unrolled not defined

It seems this was renamed to mul_unrolled in b028a58

@c42f
Copy link
Member

c42f commented Jun 5, 2019

Thanks for fixing this, by the way! It would be ideal if we were benchmarking systematically but nobody has had the impetus to make good infrastructure for that yet.

it could be interesting to have a fake mini-benchmark running to be sure the functions are fine

If you can find a way to run a dummy version of these from runtests.jl that would at least make sure we're testing the code. The key being to keep them as quick as possible in dummy mode (small iteration count / small max size) and not disturb the runtests output (maybe have a way to pass an IO to redirect output).

@c42f
Copy link
Member

c42f commented Jul 13, 2019

What's the status of this PR? Is it ready for a closer look or is there more work to do?

@matbesancon
Copy link
Contributor Author

@c42f yup good to go, pulling off something for the run tests should come in another one IMO

Copy link
Member

@c42f c42f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loop in run_benchmarks.jl appears broken by these changes. Other than that the changes look sensible.

Feel free to delete the FixedSizeArrays sections entirely — that package hasn't been maintained for ages and we've also deprecated and delete the StaticArrays.FixedSizeArrays compatibility layer.


for n = 2:16
N = n
for N = 2:16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just tried it, and this doesn't work (at least with benchmark2.jl and julia-1.1) presumably due to scoping rules: inside benchmark2.jl N == 4 always.

Copy link
Member

@c42f c42f Jul 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After hacking around this locally, I also get the following when running run_benchmarks.jl

=====================================
    Benchmarks for 14×14 matrices
=====================================
ERROR: LoadError: LoadError: conversion to pointer not defined for SizedArray{Tuple{14,14},Float64,2,2}
Stacktrace:
 [1] error(::String) at ./error.jl:33
 [2] unsafe_convert(::Type{Ptr{Float64}}, ::SizedArray{Tuple{14,14},Float64,2,2}) at ./pointer.jl:67
 [3] macro expansion at /home/tcfoster/.julia/dev/StaticArrays/src/matrix_multiply.jl:307 [inlined]
 [4] mul_blas!(::Size{(14, 14)}, ::SizedArray{Tuple{14,14},Float64,2,2}, ::Size{(14, 14)}, ::Size{(14, 14)}, ::SizedArray{Tuple{14,14},Float64,2,2}, ::SizedArray{Tuple{14,14},Float64,2,2}) at /home/tcfoster/.julia/dev/StaticArrays/src/matrix_multiply.jl:289
 [5] macro expansion at /home/tcfoster/.julia/dev/StaticArrays/src/matrix_multiply.jl:266 [inlined]
 [6] _mul! at /home/tcfoster/.julia/dev/StaticArrays/src/matrix_multiply.jl:248 [inlined]
 [7] mul! at /home/tcfoster/.julia/dev/StaticArrays/src/matrix_multiply.jl:17 [inlined]
 [8] macro expansion at /home/tcfoster/.julia/dev/StaticArrays/perf/benchmark2.jl:27 [inlined]
 [9] f_mut_array(::Int64, ::SizedArray{Tuple{14,14},Float64,2,2}) at /home/tcfoster/.julia/dev/StaticArrays/perf/benchmark2.jl:27
 [10] top-level scope at none:0
 [11] include at ./boot.jl:326 [inlined]
 [12] include_relative(::Module, ::String) at ./loading.jl:1038
 [13] include(::Module, ::String) at ./sysimg.jl:29
 [14] include(::String) at ./client.jl:403
 [15] top-level scope at /home/tcfoster/.julia/dev/StaticArrays/perf/run_benchmarks.jl:6 [inlined]
 [16] top-level scope at ./none:0
 [17] include at ./boot.jl:326 [inlined]
 [18] include_relative(::Module, ::String) at ./loading.jl:1038
 [19] include(::Module, ::String) at ./sysimg.jl:29
 [20] exec_options(::Base.JLOptions) at ./client.jl:267
 [21] _start() at ./client.jl:436

@matbesancon
Copy link
Contributor Author

Bump here.
Also, we did the following thing for Distributions: JuliaStats/Distributions.jl#1000
Just run one of the bencmarks if CI is active

@c42f
Copy link
Member

c42f commented Oct 24, 2019

See comments further up - I tried this stuff out a while ago and I think this PR is currently still broken code?

I'd definitely support connecting to CI and quickly running a single sample to keep the benchmarks working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants