Add benchmark suite #8
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8 +/- ##
=======================================
Coverage 91.42% 91.42%
=======================================
Files 2 2
Lines 70 70
=======================================
Hits 64 64
Misses 6 6 ☔ View full report in Codecov by Sentry. |
| using PkgBenchmark, FastCholesky, Dates; \ | ||
| results = benchmarkpkg("FastCholesky"; script="benchmarks/speed/benchmarks.jl"); \ | ||
| export_markdown("benchmarks/speed/exports/benchmark-"*Dates.format(Dates.now(), "yyyy-mm-ddTHH-MM")*".md", results); \ | ||
| Base.Filesystem.rm("benchmark"; force=true, recursive=true)' |
There was a problem hiding this comment.
This line gave me a bit of bf
- I didn't notice that there is no
sat the end of the folder, I though this line would just remove the entire benchmark - It is not really used after all? There is no folder called
benchmarkand the output is also in the different place
| Base.Filesystem.rm("benchmark"; force=true, recursive=true)' |
src/FastCholesky.jl
Outdated
| "`fastcholesky!` is not defined for `UniformScaling`. The shape is not determined." | ||
| ) | ||
| end | ||
| fastcholesky(x::UniformScaling) = sqrt(x.λ) * I |
There was a problem hiding this comment.
I think this operation should throw, because this behaviour is not compatible with the output of the other types. fastcholesky must return a Cholesky factorization in the form of the Cholesky structure, but here it simply returns a squared-root of the matrix. For this operation we have a different method called cholsqrt.
There was a problem hiding this comment.
Great start @bartvanerp!
I noticed some unused lines of code in the Makefile (I guess copied from RxInfer, they are not needed here as the codebase is much simpler). Also read other comments of mine.
I'm also not sure about the change for the fastcholesky! for the UniformScaling, we can discuss.
|
I played around with the benchmark, @bartvanerp please check if you agree with the changes. |
Added benchmark suite to the package