Make the rearranged Jacobian opt-in - #524
Open
oameye wants to merge 1 commit into
Open
Conversation
`get_harmonic_equations` derived the Jacobian of the rearranged system on every call. The rearrangement is a symbolic mass-matrix inversion whose cost grows combinatorially with the number of harmonics, and it dominates the runtime of this function on larger systems. Nothing in the stability analysis reads the field: `HarmonicSteadyState` evaluates the Jacobian implicitly, solving for the derivatives numerically once the parameters have values. That agrees with the rearranged matrix at every steady state, which is the only place it is evaluated. Rename the kwarg `jacobian` to `explicit_jacobian` and default it to `false`, so deriving the matrix is something you ask for when you want to inspect it.
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
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.
get_harmonic_equationsderived the Jacobian of the rearranged system on every call. The rearrangement is a symbolic mass-matrix inversion whose cost grows combinatorially with the number of harmonics, and on larger systems it dominates the runtime of the whole function. A van der Pol ansatz in three harmonics is already prohibitive, and a two-tone Duffing with nonlinear damping produces a 366,698-node expression that LLVM then needs about ten minutes to compile on first use.Nothing in the stability analysis reads the field.
HarmonicSteadyStateevaluates the Jacobian implicitly, solving for the derivatives numerically once the parameters have values. That agrees with the rearranged matrix at every steady state, which is the only place the Jacobian is ever evaluated: away from one the two differ by∂(M⁻¹)/∂u ⋅ f(u), which vanishes whenf(u) = 0.So deriving the matrix becomes something you ask for when you want to look at it:
jacobianis renamed toexplicit_jacobianand now defaults tofalseget_Jacobian(eom)remains available for computing it on demandBreaking
This is a breaking change on two levels.
The kwarg rename is loud:
get_harmonic_equations(deq; jacobian=true)now raises aMethodErrorrather than silently doing something different. That is deliberate, and it covers everyone who passed the kwarg explicitly.The default flip is quiet, and worth stating plainly: code that called
get_harmonic_equations(deq)with no kwarg and then readharmonic_eq.jacobianused to get a filled symbolic matrix and now gets the NaN placeholder, with no error. There is no shim for that case; the migration isexplicit_jacobian=trueorget_Jacobian(harmonic_eq).Note that the placeholder was already the outcome whenever the symbolic solve exceeded the Bareiss budget, so callers reading that field already had to handle it.
Testing
Full suite passes. The four call sites in the test suite that passed
jacobian=were updated.