Add scipy fallbacks for generalized Lyapunov and discrete Sylvester equations#1234
Open
kwlee2025cpp wants to merge 1 commit into
Open
Add scipy fallbacks for generalized Lyapunov and discrete Sylvester equations#1234kwlee2025cpp wants to merge 1 commit into
kwlee2025cpp wants to merge 1 commit into
Conversation
…quations lyap and dlyap previously raised ControlArgument for method='scipy' (explicit, or auto-selected when slycot is absent) on three cases that SLICOT handles. Add pure scipy/numpy fallbacks for all three: - Generalized Lyapunov, continuous and discrete (E != I): congruence transform by inv(E) to a standard Lyapunov equation, then scipy's solve_continuous/discrete_lyapunov. Requires E nonsingular; SLICOT sg03ad (Penzl's generalized Schur method) also handles singular E and remains the method='slycot' path. A nonsingular-E failure raises a clear ControlArgument. Because the transform inverts E, an ill-conditioned E yields reduced accuracy; this now emits a UserWarning recommending method='slycot' (the continuous path was previously silent, while the discrete path warned only incidentally). - Discrete Sylvester (A X Q^T - X + C = 0): Bartels-Stewart method via complex Schur factors of A and Q^T with column-by-column triangular solves, O(n^3 + m^3), matching the Hessenberg-Schur cost of SLICOT sb04qd. Includes the solvability check that no eigenvalue pair of A and Q^T has product (almost) equal to 1. Also fix two incorrect slycot import-fallback aliases (sb0qmd -> sb04qd, sb04ad -> sg03ad) so the names resolve to None, not NameError, when slycot is absent; and correct the dlyap Notes, which described the scipy Sylvester path as a Kronecker O((nm)^3) solve (it is now Bartels-Stewart, O(n^3 + m^3)). Tests parametrize method=[None, 'scipy', 'slycot'] and cross-check that the scipy and slycot solutions agree, and cover the nonsingular-E requirement, the ill-conditioned-E warning, and the singular discrete-Sylvester case. Co-Authored-By: Claude Fable 5 <[email protected]> Co-Authored-By: Claude Opus 4.8 <[email protected]>
2c68e3e to
c1ab92c
Compare
Member
|
@kwlee2025cpp Thanks for proposing these changes. Can you answer a few questions to help with review and consideration for merging?
I am a little concerned that this PR is just a Claude-generated thing that no one is asking for. I am also concerned we are about to get many more Claude-generated PRs as your description suggestions ("...It is the first of a |
Contributor
|
Approving the workflows because the code on first glance looks good and obviously not malicious. No assessment about scientific merit, I fully support @slivingston's inquiry. |
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.
Summary
lyapanddlyapcurrently raiseControlArgument("method='scipy' not valid for ...")for three problems, so solving them requires the optionalslycotpackage:A X Eᵀ + E X Aᵀ + Q = 0,A X Aᵀ − E X Eᵀ + Q = 0,A X Qᵀ − X + C = 0.This PR implements pure scipy/numpy fallbacks for all three, so they work
when
slycotis not installed. Whenslycotis installed it remains thedefault (
method='slycot'), and the new tests cross-validate the scipyresults against it.
What it adds
Generalized Lyapunov (continuous and discrete). The equation is reduced
to a standard Lyapunov equation by a congruence transform with
inv(E)(
à = E⁻¹A,Q̃ = E⁻¹Q E⁻ᵀ, leavingXunchanged), then solved withscipy.linalg.solve_{continuous,discrete}_lyapunov. This requiresEto benonsingular. SLICOT
SG03AD(method='slycot'), based on the generalizedSchur method of Penzl, additionally handles singular
E; that case raises aclear
ControlArgumentpointing the user tomethod='slycot'.Discrete-time Sylvester. Solved by the Bartels–Stewart method: complex
Schur factorizations of
AandQᵀ, then a column-by-column backsubstitution. This matches the
O(n³ + m³)complexity of theHessenberg–Schur algorithm in SLICOT
SB04QD(method='slycot').Singularity (a pair of eigenvalues with
λᵢ·λⱼ ≈ 1) is detected and raisesa clear
ControlArgument.Ill-conditioned
E. Because the generalized-Lyapunov fallback invertsE, accuracy degrades whenEis poorly conditioned. AUserWarningisnow issued when
cond(E) > 1/√eps, recommendingmethod='slycot'. (Thisalso surfaces a case in the continuous path that was previously silently
inaccurate.)
Fixes included
slycotguards: theexcept ImportErrorbranches bound the sentinels under the wrong names(
sb0qmdforsb04qd,sb04adforsg03ad), so on a slycot-lessinstall the intended
Nonefallbacks were never set under the names thecode references.
dlyapdocstring note (implied a Kronecker-product method);corrected to Bartels–Stewart, with Golub–Nash–Van Loan and
Bartels–Stewart references added to the
Notes/References.Testing
test_lyap_g,test_dlyap_g, andtest_dlyap_sylvester(scipy result matches slycot to~1e-9…1e-16).test_lyap_g_ill_conditioned_E_scipy(parametrized overlyap/dlyap)asserts the ill-conditioned-
EUserWarning.test_dlyap_sylvester_singular_scipyasserts the singular-pencilControlArgument.19 passed, 12 skipped(no slycot) /31 passed(slycot).Only
control/mateqn.pyandcontrol/tests/mateqn_test.pyare touched.Scope
This is an intentionally small, self-contained slice. It is the first of a
short series factoring a larger "scipy fallbacks for slycot-gated routines"
effort into independently reviewable PRs; the matrix-equation solvers go
first because they are foundational and easy to validate against SLICOT.
gramis the planned next slice.Alternatives considered
The same
inv(E)reduction is used by pyMOR's dense scipy Lyapunovbackend. Factoring the pencil
(A, E)withscipy.linalg.qzinstead ofinverting
E(asharolddoes) is more robust for ill-conditionedEand is a natural follow-up. Truly singular
E(descriptor systems withinfinite generalized eigenvalues) is out of scope here and continues to
require
method='slycot'; a pure-scipy path for that case would needspectral-projector deflation of the infinite spectrum.