-
Notifications
You must be signed in to change notification settings - Fork 3
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 NNlib extension #85
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #85 +/- ##
==========================================
- Coverage 79.58% 79.36% -0.22%
==========================================
Files 16 17 +1
Lines 676 698 +22
==========================================
+ Hits 538 554 +16
- Misses 138 144 +6 ☔ 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
|
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.
Missing a few second order tests but otherwise lgtm
As a side note, NNlib activation functions are implemented from Base functions, so if we supported the following properly it would be enough:
ifelse
oftype
clamp
muladd
evalpoly
@fastmath
(not sure what it takes to support it)
Of course the local sparsity rules would not kick in, or not as well
To some degree, the argument that everything is made up from Base functions can be applied to any pure Julia code. We're trying to skip some computations. I was hoping for a visible speed-up in the benchmarks, but they only test global tracing on Benchmarking locally, the difference is very minor on julia> @benchmark local_jacobian_pattern(LAYER_RELU, $INPUT_FLUX)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
Range (min … max): 16.458 μs … 2.417 ms ┊ GC (min … max): 0.00% … 98.49%
Time (median): 18.083 μs ┊ GC (median): 0.00%
Time (mean ± σ): 18.862 μs ± 38.831 μs ┊ GC (mean ± σ): 3.96% ± 1.95%
▁▄▄▃▁ ▁▂▄▅▇▆█▇▆▅▃▂▁ ▂
▃▅███████▆▇▇█████████████████▇████████▇▅██▇▇▇▇▆▆▆▂▆▅▅▆▆▄▆▆▆ █
16.5 μs Histogram: log(frequency) by time 21.1 μs <
Memory estimate: 23.91 KiB, allocs estimate: 355. julia> @benchmark local_jacobian_pattern(LAYER_RELU, $INPUT_FLUX)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
Range (min … max): 16.625 μs … 2.329 ms ┊ GC (min … max): 0.00% … 98.01%
Time (median): 18.000 μs ┊ GC (median): 0.00%
Time (mean ± σ): 18.891 μs ± 37.138 μs ┊ GC (mean ± σ): 3.76% ± 1.95%
▁▂▄▅▅▇██▇▆▄▃▂▁▁▁▁▁ ▁ ▁▁▁▁ ▂
▃▁▁▁▃▆▅▇▇████████████████████▇████████████▇█▆▇▇▆▇▆▆▇▆▆▇▆▆▆▅ █
16.6 μs Histogram: log(frequency) by time 21.4 μs <
Memory estimate: 23.78 KiB, allocs estimate: 353. For other activation functions that are more involved than |
We already support and test on SparseConnectivityTracer.jl/test/second_order.jl Lines 190 to 197 in 57def1e
Adding If we ever support them, then probably just by computing the most conservative estimate of the Jacobian/Hessian. Or we simply add the minimum required code for overloads on |
I'm not against the extension, I think it's a good idea. Just using the opportunity to highlight that we're missing some stuff from Base, but #74 is perfect to track it. Feel free to merge! |
I just realized another upside of adding extensions: it allows non-dual tracers to run through functions that otherwise call |
They should already be able to flow through |
You're right about |
Adds NNlib activation functions, as discussed in this comment in #56.