Add SpecialFunctions extension #82
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The problematic The problem is that you are invoking The design of SCT us problematic. Instead of defining new methods in the calling module, it defines new methods in SCR. The better way to do this would be to define a macro to extend methods in SCT but define those methods in the calling module. |
|
I put this eval step in the initialization precisely because the error was warning me not to eval stuff during precompilation 🤔 What do you mean by "define new methods in the calling module"? Should I use the eval of Base/SpecialFunctions? Or is the calling module yet another one? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #82 +/- ##
==========================================
+ Coverage 76.19% 77.49% +1.29%
==========================================
Files 14 16 +2
Lines 689 662 -27
==========================================
- Hits 525 513 -12
+ Misses 164 149 -15 ☔ View full report in Codecov by Sentry. |
|
I finally managed to get it working. Here's a summary of the changes: Source
Extensions
Tests
|
Merged
adrhill
reviewed
May 22, 2024
ext/SparseConnectivityTracerSpecialFunctionsExt/SparseConnectivityTracerSpecialFunctionsExt.jl
Show resolved
Hide resolved
Comment on lines
+1
to
+30
| function overload_all(M) | ||
| exprs_1_to_1 = [ | ||
| quote | ||
| $(overload_connectivity_1_to_1(M, op)) | ||
| $(overload_gradient_1_to_1(M, op)) | ||
| $(overload_hessian_1_to_1(M, op)) | ||
| end for op in nameof.(list_operators_1_to_1(Val(M))) | ||
| ] | ||
| exprs_2_to_1 = [ | ||
| quote | ||
| $(overload_connectivity_2_to_1(M, op)) | ||
| $(overload_gradient_2_to_1(M, op)) | ||
| $(overload_hessian_2_to_1(M, op)) | ||
| end for op in nameof.(list_operators_2_to_1(Val(M))) | ||
| ] | ||
| exprs_1_to_2 = [ | ||
| quote | ||
| $(overload_connectivity_1_to_2(M, op)) | ||
| $(overload_gradient_1_to_2(M, op)) | ||
| $(overload_hessian_1_to_2(M, op)) | ||
| end for op in nameof.(list_operators_1_to_2(Val(M))) | ||
| ] | ||
| return quote | ||
| $(exprs_1_to_1...) | ||
| $(exprs_2_to_1...) | ||
| $(exprs_1_to_2...) | ||
| end | ||
| end | ||
|
|
||
| eval(overload_all(:Base)) |
There was a problem hiding this comment.
Yeah, this combined with list_operators_* and the big quotes in overload_*.jl feels very hacky. 🫤
There are too many layers of metaprogramming with a lot of implicit dependencies in their design that interact across several files.
There was a problem hiding this comment.
It's exactly the way they do it in ForwardDiff:
- The big quotes: https://github.com/JuliaDiff/ForwardDiff.jl/blob/1907196ba50e3eb337860301ece3ec37d0582257/src/dual.jl#L230-L244
- The evaluation: https://github.com/JuliaDiff/ForwardDiff.jl/blob/1907196ba50e3eb337860301ece3ec37d0582257/src/dual.jl#L469-L472
- The listing: https://github.com/JuliaDiff/ForwardDiff.jl/blob/1907196ba50e3eb337860301ece3ec37d0582257/src/dual.jl#L463
There was a problem hiding this comment.
I spent quite a lot of time trying out various things to make it work with minimal changes, and this seems to work.
There was a problem hiding this comment.
What's in the quotes is exactly the same as what was directly eval-ed earlied (if you ignore the additional module prefixes, which I got from ForwardDiff).
The difference is that we split code generation (creating the expression) from code evaluation (running it through eval)
There was a problem hiding this comment.
The contents of overload_all are only designed so that it can be a one-liner in all the package extensions we will no doubt need to add. It's nothing but a big concatenation of all the expressions
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 versioninfoTargetBaselineTarget 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 versioninfoBaseline 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 versioninfoRuntime information
|
Would it be easier to implement this as a macro? A macro is a function that returns an expression and calls |
That's probably a good idea, will try it in a later PR! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
At the moment, the tests fail because I try to "
evalinto a closed module during precompilation".For anyone who wants to take a look:
overload_allcalls multiple functions that define specific overloads with@evalSparseConnectivityTracer.jl/src/overload_connectivity.jl
Lines 13 to 24 in c49dc32
I would love some guidance on:
evals (duringinitor during precompilation)@evalor the functionevaleval