switch to SparseConnectivityTracer and DifferentiationInterface #78
Conversation
This file contains hidden or 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
added 4 commits
August 4, 2025 23:16
…ntiationInterface in jacobian preparation in NonlinearOperator
…arseConnectivityTracer, could removed Symbolics as a main dependence now, version bump + changelog
chmerdon
force-pushed
the
chmerdon/diffinterface
branch
from
f4f4a38 to
c5b24b6
Compare
|
Hey there! Just popping by to ask how the transition went? |
j-fu
reviewed
Aug 6, 2025
| @@ -8,6 +8,7 @@ using Metis | |||
| using Aqua | |||
| using Triangulate | |||
| using SimplexGridFactory | |||
| using Symbolics | |||
|
|
|||
There was a problem hiding this comment.
Pull Request Overview
This pull request modernizes the automatic differentiation infrastructure by switching from SparseDiffTools and Symbolics to SparseConnectivityTracer and DifferentiationInterface for sparsity detection and jacobian preparation.
- Replaces Symbolics.jacobian_sparsity with SparseConnectivityTracer's jacobian_sparsity function
- Refactors NonlinearOperator to use DifferentiationInterface for jacobian computation with sparse backends
- Adds new autodiff_backend parameter for configurable automatic differentiation backends
Reviewed Changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ExtendableFEM.jl | Updates imports to use new AD packages and removes old dependencies |
| src/common_operators/nonlinear_operator.jl | Major refactor of KernelEvaluator and jacobian computation using DifferentiationInterface |
| src/common_operators/bilinear_operator.jl | Replaces Symbolics sparsity detection with SparseConnectivityTracer |
| src/common_operators/bilinear_operator_dg.jl | Replaces Symbolics sparsity detection with SparseConnectivityTracer |
| test/runtests.jl | Adds Symbolics import for test compatibility |
| CHANGELOG.md | Documents the changes and new parameter |
| else | ||
| jac_sparsity_pattern = O.parameters[:sparse_jacobians_pattern] | ||
| end | ||
| jac = Tv.(sparse(sparsity_pattern)) |
There was a problem hiding this comment.
The sparse_jacobians variable is hardcoded to false, which contradicts the parameter O.parameters[:sparse_jacobians] used earlier in the function. This will prevent sparse jacobian functionality from working correctly.
Suggested change
| jac = Tv.(sparse(sparsity_pattern)) | |
| sparse_jacobians = O.parameters[:sparse_jacobians] | |
| if sparse_jacobians | |
| if O.parameters[:sparse_jacobians_pattern] === nothing | |
| jac_sparsity_pattern = DifferentiationInterface.sparsity_pattern(jac_prep) | |
| else | |
| jac_sparsity_pattern = O.parameters[:sparse_jacobians_pattern] | |
| end | |
| jac = Tv.(sparse(jac_sparsity_pattern)) |
Copilot uses AI. Check for mistakes.
| else | ||
| jac_sparsity_pattern = O.parameters[:sparse_jacobians_pattern] | ||
| end | ||
| jac = Tv.(sparse(sparsity_pattern)) |
There was a problem hiding this comment.
The variable 'sparsity_pattern' is undefined in this scope. It should be 'jac_sparsity_pattern' which is defined above.
Suggested change
| jac = Tv.(sparse(sparsity_pattern)) | |
| jac = Tv.(sparse(jac_sparsity_pattern)) |
Copilot uses AI. Check for mistakes.
| @@ -8,6 +8,7 @@ using Metis | |||
| using Aqua | |||
| using Triangulate | |||
| using SimplexGridFactory | |||
| using Symbolics | |||
There was a problem hiding this comment.
Adding Symbolics as a test dependency contradicts the PR's purpose of removing Symbolics as a main dependency. Consider refactoring tests to use the new SparseConnectivityTracer instead.
Suggested change
| using Symbolics |
Copilot uses AI. Check for mistakes.
|
Bin im Urlaub. Für mich ok wenn die Tests laufen |
…ependencies section in doc
|
It currently breaks one of my project examples, similar to Example330, which fails in the docs. I will look into it. |
|
@pjaap thanks! @gdalle thanks for popping by and the great packages! So far the transition went well, we just have a problem with some examples were we need to sparse-detect and differentiate functionals that are ForwardDiff jacobians by themself, see e.g. here. I also did not yet compare runtimes and allocations and other backends than ForwardDiff, but will do this later maybe. @j-fu Symbolics is still needed in some of the examples (some have tests) where given functions are differentiated symbolically to construct analytic benchmarks. Dann erstmal schönen Urlaub! Ich bin auch noch im Urlaub. |
|
@chmerdon do you have an MWE demonstrating the failure you're talking about? What code should I run and in which environment? What's the error message? |
|
Sorry for the delay, I fell sick last week.... Here is a MWE for the problem. It is a strip-down of Example330. Basically, it should simply minimize with the optimal solution In our FE Library, we solve weak formulations, hence we solve with for the linear operator. This operator is computed automatically by ForwardDiff here. module MWE
using ExtendableFEM
using ExtendableGrids
using DiffResults
using ForwardDiff
function nonlinear_F!(result, input, qpinfo)
result[1] = input[1]^2 - 2*input[1] # => input = 1 is minimizer
end
## derivative of nonlinear_F via AD
function nonlinear_DF!()
Dresult = nothing
cfg = nothing
result_dummy = nothing
F(qpinfo) = (a, b) -> nonlinear_F!(a, b, qpinfo)
return function closure(result, input, qpinfo)
if Dresult === nothing
## first initialization of DResult when type of input = F is known
result_dummy = zeros(eltype(input), 1)
Dresult = DiffResults.JacobianResult(result_dummy, input)
cfg = ForwardDiff.JacobianConfig(F(qpinfo), result_dummy, input, ForwardDiff.Chunk{length(input)}())
end
Dresult = ForwardDiff.vector_mode_jacobian!(Dresult, F(qpinfo), result_dummy, input, cfg)
copyto!(result, DiffResults.jacobian(Dresult))
return nothing
end
end
function main()
## define unknowns
u = Unknown("u"; name = "potential")
## define problem
PD = ProblemDescription()
assign_unknown!(PD, u)
assign_operator!(PD, NonlinearOperator(nonlinear_DF!(), [id(u)]; sparse_jacobians = false))
## simple 2D grid
xgrid = grid_unitsquare(Triangle2D)
## solve
FES = FESpace{H1P1{1}}(xgrid)
sol = solve(PD, FES; maxiterations = 20)
# should be all ≈ 1
@show sol[u].entries
return sol
end
end # module |
|
The MWE works fine on current master, but with SCT we get the following error: |
|
@adrhill can you maybe take a look? |
|
The stack trace is odd, ForwardDiff's Where and how do you call SCT? |
|
In case you are attempting to obtain a second-order sparsity pattern by using SCT to trace through a ForwardDiff computation, you might just want to use SCT's |
|
Unless the two differentiations are with respect to different variables? |
|
Thanks for your input. I think we'll continue the discussion once @chmerdon and me are back from vacation in September 😄 |
|
Same here, so that works well for me 😂 |
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.
sparsity detection for BilinearOperator and jacobian preparation for NonlinearOperator now is done with SparseConnectivityTracer and DifferentiationInterface (as suggested by issue #50); SparseDiffTools and Symbolics removed as a main dependency