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

WIP/RFC: Run LinearAlgebra tests with --compile=min. #34464

Closed
wants to merge 1 commit into from

Conversation

maleadt
Copy link
Member

@maleadt maleadt commented Jan 21, 2020

Implements JuliaLang/LinearAlgebra.jl#690. Has the same effect as #34456, too. Current approach introduces some non-determinism (whether tests are executed on the --compile=min worker or not), so that may not be wanted. Unless that flag is supposed to be non-breaking, in which case this would add some coverage?

@maleadt maleadt added the testsystem The unit testing framework and Test stdlib label Jan 21, 2020
@maleadt maleadt requested a review from Keno January 21, 2020 14:16
@maleadt maleadt force-pushed the tb/test_compile_min branch from 1014383 to 6c34f6b Compare January 21, 2020 15:03
@maleadt
Copy link
Member Author

maleadt commented Jan 21, 2020

Some LinAlg tests take ages with --compile=min; looking into it.

@maleadt maleadt force-pushed the tb/test_compile_min branch from 6c34f6b to 948221d Compare January 21, 2020 16:51
@maleadt
Copy link
Member Author

maleadt commented Jan 21, 2020

Doesn't look as clear-cut as expected:

 "test"                                  "regular"       "compile=min"  ""           
 "LinearAlgebra/pinv"                   5.58             inf            "inf"
 "LinearAlgebra/qr"                    43.86          257.27            "586.57%"    
 "LinearAlgebra/dense"                 80             453.33            "566.66%"    
 "LinearAlgebra/bunchkaufman"          25.1           139.36            "555.22%"    
 "LinearAlgebra/givens"                 8.43           31.3             "371.29%"    
 "LinearAlgebra/eigen"                  9.92           23.52            "237.10%"    
 "LinearAlgebra/triangular"           432.41         1010.43            "233.67%"    
 "LinearAlgebra/lq"                    40.95           84.52            "206.40%"    
 "LinearAlgebra/schur"                 10.46           17.03            "162.81%"    
 "LinearAlgebra/special"               82.43          112.22            "136.14%"    
 "LinearAlgebra/lu"                    59.32           70.87            "119.47%"    
 "LinearAlgebra/diagonal"             122.83          138.03            "112.37%"    
 "LinearAlgebra/lapack"                22.66           23.27            "102.69%"    
 "LinearAlgebra/generic"               26.78           25.01            "93.39%"     
 "LinearAlgebra/symmetric"             66.47           52.74            "79.34%"     
 "LinearAlgebra/cholesky"              39.22           29.86            "76.13%"     
 "LinearAlgebra/tridiag"               28.34           21.04            "74.24%"     
 "LinearAlgebra/structuredbroadcast"   35.16           21.43            "60.95%"     
 "LinearAlgebra/bidiag"               109.63           66.43            "60.59%"     
 "LinearAlgebra/blas"                  21.78           12.99            "59.64%"     
 "LinearAlgebra/svd"                   30.93           16.95            "54.80%"     
 "LinearAlgebra/matmul"               165.49           88.3             "53.36%"     
 "LinearAlgebra/uniformscaling"        26.51            9.75            "36.78%"     
 "LinearAlgebra/hessenberg"            48.38           17.01            "35.16%"     
 "LinearAlgebra/adjtrans"              22.75            4.82            "21.19%"     
 "LinearAlgebra/addmul"               656.88           37.92            "5.77%"

cc @andreasnoack

@maleadt maleadt changed the title Run LinearAlgebra tests with --compile=min. WIP/RFC: Run LinearAlgebra tests with --compile=min. Jan 21, 2020
@maleadt maleadt added the DO NOT MERGE Do not merge this PR! label Jan 21, 2020
@JeffBezanson
Copy link
Member

We probably don't really want to run linalg tests with --compile=min, since it's just the sort of code you want optimized, so we should test the path that's usually used. It would also require using very small matrix sizes everywhere, but that's doable I suppose.

@maleadt
Copy link
Member Author

maleadt commented Jan 21, 2020

Some tests, like LinearAlgebra/addmul, are pretty brute-forcey and don't really reflect actual usage (#34456 (comment)). Not compiling those dramatically lowers test times, e.g. on AArch64 from around 2500 to 5000 seconds (!!) to only 250.

@JeffBezanson
Copy link
Member

I know it takes a long time, but how confident are we that no change in LLVM or our codegen will ever break those tests?

@maleadt
Copy link
Member Author

maleadt commented Jan 21, 2020

I know it takes a long time, but how confident are we that no change in LLVM or our codegen will ever break those tests?

Maybe only on master and not on PRs then? Or, if we ever would decide to use bors (say, to fix JuliaCI/julia-buildbot#126) we could have different behavior on the trying/staging branches.

@andreasnoack
Copy link
Member

Interesting to see the numbers. I'm really surprised that the triangular tests are slower with compile=min.

@maleadt
Copy link
Member Author

maleadt commented Jan 22, 2020

Anyway, with exception of LinearAlgebra/addmul this doesn't seem worth the change.

@maleadt maleadt closed this Jan 22, 2020
@StefanKarpinski
Copy link
Member

Between this and some no-specialized experiments that Jameson did last year, we seem to have one lesson: simple "just don't compile" changes are not as effective as one might expect. The reliance on the speed of JIT code is too significant and diffuse to address like this. That points to really needing something significantly more clever like a tiered JIT approach in order to get the best of both worlds.

@JeffBezanson
Copy link
Member

That's right, but part of it is just that thoroughly testing a large amount of linear algebra code plus its compiler takes a long time. I don't think anybody believes you can test e.g. lapack or a C++ compiler in a couple minutes. The main question is what are we trying to test here? If we want to test LinearAlgebra and our compiler on that kind of code as thoroughly as possible, I suspect this is the cost. But if making CI faster is more important, the best approach would be to cut down how many cases we test (e.g. fewer element types, fewer/smaller matrix sizes, a sparser sampling of argument combinations, etc.)

In any case the numbers here are indeed interesting!

@DilumAluthge DilumAluthge deleted the tb/test_compile_min branch March 25, 2021 21:57
@DilumAluthge DilumAluthge removed the DO NOT MERGE Do not merge this PR! label Jun 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
testsystem The unit testing framework and Test stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants