Closed gdalle closed 3 weeks ago
Thanks @gdalle! Can I finalize the PR for SCT.jl after this one?
Sure, it will be even easier to finalize then, just switch the detector object. The only thing you have to do is figure out if you want to pass this detector all the way down the call chain, e.g. by making it part of one of your structures.
I suspect the doc failure is due to the ungodly way in which you handle Symbolics. It's not in your Project.toml but it's still a dependency somehow. How does that work? It's probably due to your well-known and unjustified hatred of package extensions, but really this is a terrible footgun.
Oops, for SymbolicsSparsityDetector
to exist I need the latest version of Symbolics, but they have dropped support for Julia 1.6 a long time ago. If you check the CI log on main, you'll see that you're using an old version of Symbolics.
Note that SparseConnectivityTracer.jl supports Julia 1.6 ;)
I propose to update the compat entry of Julia to v1.9 such we have support for package extensions. The next release will be breaking in all cases because SCT.jl will be the default. I must also rename the Symbolics backends.
In that case it probably doesn't make much sense to try hard to support Symbolics again. Maybe you can just rebase your SCT PR on this one, switch the detector and we drop Symbolics once and for all?
In that case it probably doesn't make much sense to try hard to support Symbolics again. Maybe you can just rebase your SCT PR on this one, switch the detector and we drop Symbolics once and for all?
Yes, let's do that.
Closing in favor of #230
ADTypes.jl v1.0 introduced an abstract sparse autodiff interface for sparsity pattern detection and matrix coloring, which can be implemented by various packages. The latest release of Symbolics.jl, namely v5.29, includes a
SymbolicsSparsityDetector
object which satisfies the ADTypes.jl requirements of sparsity pattern detection.My PR changes your sparsity detection routines to use this detector object, in order to make it easier to switch later on. For instance, you may want to use
TracerSparsityDetector
from SparseConnectivityTracer.jl instead (see #229 and #230). The only caveat is that it uses a random vector of Lagrange multiplier, but I think that's correct since the probability of accidental cancellation between constraints is basically zero?I also removed the compat tweak on SymbolicUtils.jl in the test environment, since it has released a v2.0 which apparently fixes #232.
Ping @amontoison