Remove SparseDiffTools.jl as dependency#137
Remove SparseDiffTools.jl as dependency#137mx-p9a wants to merge 2 commits intobyuflowlab:masterfrom
Conversation
| function autodiff_jacobian!(jacob, residual!, x, p, constants) | ||
| f! = (r, x) -> residual!(r, x, p, constants) | ||
| y = constants.resid | ||
| prep = prepare_jacobian(f!, y, SPARSE_AD_BACKEND, x) |
There was a problem hiding this comment.
The whole point of preparation is to do it just once and reuse it several times. Is there any way to store the prep object you get here to avoid recomputing it? This is especially important for sparse AD, where sparsity detection is very expensive
There was a problem hiding this comment.
That's a good point. Storing the prep object shouldn't be too difficult, I'll just need to check what the optimal behavior is. I don't know how the sparsity pattern changes from function call to function call. I remember correctly, SparseConnectivityTracer captures the full sparsity pattern for a given branch in the code, right? I'll need to check in the code if that's going to create an overly dense matrix.
There was a problem hiding this comment.
That makes sense. I'll defer to @Cardoza2 on how he would like to manage this. Perhaps could be part of the constants object?
There was a problem hiding this comment.
Yeah, that seems like a decent place for it.
|
Suspect the reason for CI failure is that byuflowlab/ImplicitAD.jl#24 needs to get merged first (with new release). |
|
Oh yeah, duh... that makes sense. I'll run the tests on ImplicitAD and make sure things are good over there, then rerun the tests here. |
Replace SparseDiffTools.jl with DifferentiationInterface.jl
Closes #136. Note: requires byuflowlab/ImplicitAD.jl#24.
Summary
Removes the deprecated
SparseDiffTools.jldependency and replaces it withDifferentiationInterface.jl(v0.7) +SparseConnectivityTracer.jl+SparseMatrixColorings.jl. Also replacesDifferentialEquations.jlwithOrdinaryDiffEqin the test environment.Motivation
SparseDiffTools.jlis deprecated and causes version conflicts with modern SciML packages (specificallySciMLOperators >= 1required by recentOrdinaryDiffEq). The replacement ecosystem isDifferentiationInterface.jlwith sparse AD support viaSparseConnectivityTracer.jlandSparseMatrixColorings.jl.Changes
Project.tomlSparseDiffToolsDifferentiationInterface(0.7),SparseConnectivityTracer(1),SparseMatrixColorings(0.4)ImplicitAD(1),SciMLBase(2.117-2)src/GXBeam.jlimport SparseDiffToolswithusing DifferentiationInterface,import SparseConnectivityTracer,import SparseMatrixColoringssrc/analyses.jlThree functions were replaced:
jacobian_colors()-- Deleted. This was dead code (never called anywhere, referenced undefined variables).autodiff_jacobian!()-- ReplacedSparseDiffTools.forwarddiff_color_jacobian!with DI'sAutoSparse(AutoForwardDiff(); ...)backend usingprepare_jacobian/jacobian!. This is actually an improvement: the old code passedcolorvec = 1:length(x)(dense coloring, never exploiting sparsity), while the new code automatically detects and exploits the sparsity pattern viaSparseConnectivityTracer.matrixfree_jacobian()-- ReplacedSparseDiffTools.JacVecwith DI'sprepare_pushforward/pushforward!wrapped in aLinearMap. This preserves the exact same interface (an operator supportingmul!) compatible withIterativeSolvers.gmres!.Test environment
DifferentialEquations.jlwithOrdinaryDiffEq(the tests only useRodas4()andDABDF2(), both from OrdinaryDiffEq)BenchmarkToolsfor the benchmark scriptBenchmark Results
Benchmark script at
benchmark/benchmark.jlusingBenchmarkTools.jl(10 samples each):Static Joined-Wing (40 elements, 141 nonlinear load steps) --
matrixfree_jacobianEigenvalue Analysis (40 elements, 18 sweeps x 3 RPMs) --
autodiff_jacobian!Time-Domain Wind Turbine (5 elements, 101 time steps) --
matrixfree_jacobianPerformance is identical within measurement noise. Memory usage is unchanged.
Dependency note
This PR requires ImplicitAD.jl#XX which removes the unused
DifferentiationInterfacedependency from ImplicitAD (it was listed as a dep but never imported in any source file). Without that change, ImplicitAD'sDI = "0.6"compat bound prevents resolving DI 0.7.Test results
All 33 test groups pass, 0 failures.