-
Notifications
You must be signed in to change notification settings - Fork 5
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
Generalize sparsity pattern representations #119
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #119 +/- ##
==========================================
- Coverage 89.10% 87.96% -1.14%
==========================================
Files 31 31
Lines 1202 1354 +152
==========================================
+ Hits 1071 1191 +120
- Misses 131 163 +32 ☔ View full report in Codecov by Sentry. |
Benchmark resultJudge resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
Benchmark resultJudge resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
Benchmark resultJudge resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
Looks like
|
|
In 2ec6489, the full suite passed in 30 minutes... |
A description of our compile time problems (cause & solution) can be found here: #120 (comment). The fix is commit 875d90d. |
Benchmark resultJudge resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
Benchmark resultJudge resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
Looks like there is a minor decrease in performance in Technically speaking, we don't need |
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.
looks good to me apart from some minor comments
I double-checked and the inlining makes no difference on first-order tracers:
first jacobian: 0.756304 seconds (729.08 k allocations: 48.768 MiB, 0.66% gc time, 99.98% compilation time)
second jacobian: 0.000062 seconds (2.05 k allocations: 169.844 KiB) without first jacobian: 0.746296 seconds (846.85 k allocations: 54.726 MiB, 0.76% gc time, 99.97% compilation time)
second jacobian: 0.000059 seconds (2.05 k allocations: 169.844 KiB) |
your call |
Since there is no cost in performance, let's just bring it back so we don't have to worry about it in the future! |
Benchmark resultJudge resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsA ratio greater than
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfoTarget
Baseline
Target resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Baseline resultBenchmark Report for /home/runner/work/SparseConnectivityTracer.jl/SparseConnectivityTracer.jlJob Properties
ResultsBelow is a table of this job's results, obtained by running the benchmarks.
Benchmark Group ListHere's a list of all the benchmark groups executed by this job:
Julia versioninfo
Runtime information
|
Closes #114.
Closes #80.
This PR relaxes tracer field types and adds an additional layer of dispatch to
X_tracer_Y_to_Z
functions by addingX_tracer_Y_to_Z_inner
, which dispatches on the field type.It also adds a generic
isempty
field to tracers, which is handled inX_tracer_Y_to_Z
, agnostic to the field types.To use custom tracers, both the native interface and the ADTypes.jl interface now expect a full tracer type to be passed, e.g.
TracerSparsityDetector(gradient_tracer_type, hessian_tracer_type)
.This is therefore tagged as a breaking release.
This PR has also lead to the discovery of compile time problems, which have been described here: #120 (comment)
To catch this, some tests have been updated and refactored.
Outdated PR description
This PR adds a layer of Abstraction by introducing
AbstractSparsityPattern
s.Over the last dozens of PRs, we've kept refactoring the Tracer type signatures to accommodate new representations for sparse vectors and sparse matrices, most notably:
AbstractSet
interface (Add RecursiveSet and DuplicateVector + better benchmarking of Brusselator #50)The introduction of flexible first- and second-order sparsity pattern abstractions in
AbstractFirstOrderPattern
andAbstractSecondOrderPattern
should facilitate shared sets (#107) and allow for the reintroduction of second-orderDictIndexset
s in the future.Recommended reading order for reviews:
indexsets.jl
(could use a better name)tracers.jl