Repository navigation
Replace exact and absolute comparisons in isunstable, baltrunc, delayd_ss and thiran - #1083
Merged
Merged
Conversation
…d_ss and thiran - isunstable: in discrete time, compare the pole magnitudes with 1 instead of with the multiplicity tolerance, and check repeated poles on the unit circle instead of on the imaginary axis. - baltrunc: default atol = 0, since the Hankel singular values scale with the gain of the system. Handle the case where all singular values are removed. - delayd_ss: test that τ/Ts is an integer to within a relative tolerance instead of requiring τ - n*Ts == 0. - thiran: round τ/Ts to the nearest integer when it is within a relative tolerance, so that rounding errors do not increase the filter order. Part of #1081 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HigNnun4aJKMoWmY7wyULn
|
This is an automated message.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1083 +/- ##
==========================================
+ Coverage 91.73% 91.74% +0.01%
==========================================
Files 42 42
Lines 5865 5873 +8
==========================================
+ Hits 5380 5388 +8
Misses 485 485 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
baggepinnen
referenced
this pull request
Oct 13, 2026
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HigNnun4aJKMoWmY7wyULn
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.








Part of #1081.
This PR replaces four exact or absolute comparisons in ControlSystemsBase that produce wrong results or errors for realistic systems.
1.
isunstablein discrete timeThe discrete-time branch compared the pole magnitudes with the multiplicity tolerance
tolp(of the order of100ϵ) instead of with 1, so every discrete-time system with a nonzero pole was reported as unstable. In addition, the loop that detects repeated poles was shared with the continuous-time branch and searched for repeated poles on the imaginary axis, so a double pole atz = 0(e.g.z^-2) was reported as unstable, while a double pole atz = -1was detected only through the first error.Change: the discrete-time branch searches for poles on the unit circle (
abs(abs(p) - 1)below the same prefilter tolerance as the continuous-time branch) and counts the multiplicity withcount_eigval_multiplicityaround the projectionp/abs(p)onto the unit circle. The final test isabs(p) > 1 + tolp. Simple poles on the unit circle (marginal stability) are classified as stable, repeated poles on the unit circle (atz = 1,z = -1or a complex pair) as unstable, consistent with the docstring and with the continuous-time branch. The continuous-time branch is unchanged; its known weaknesses are listed in #1081.2.
baltruncdefaultatolThe default
atol = sqrt(eps())is an absolute threshold on the Hankel singular values, which scale with the gain of the system.Change: the default is
atol = 0. The truncation is then determined by the relative tolerancertoland by the relative truncation to a minimal realization inbalreal(sqrt(eps)*σmax). The relative truncation is skipped when no singular values remain, so a system of zero gain returns a system with a state dimension of zero. The docstring is updated. No other function in the package callsbaltrunc.3.
delayd_ssτ - n*Ts ≈ 0is an exact test, sinceisapproxwith zero and the defaultatol = 0requires equality.Change:
isapprox(τ/Ts, n; rtol = sqrt(eps(T)))withn = round(Int, τ/Ts).4.
thiranN = ceil(Int, τ/Ts)increases the order whenτ/Tsexceeds an integer by a rounding error.Change:
D = τ/Tsis replaced byround(D)when it is within the relative tolerancesqrt(eps(T))of it, beforeN = ceil(Int, D). The docstring states the tolerance.An alternative is
N = round(Int, D), i.e.D ∈ [N-0.5, N+0.5), which Laakso et al. recommend for the Thiran filter. This would change the order of the filter for existing users whenever the fractional part ofDis below 0.5, and is therefore not included in this PR.Effect on existing users
isunstablereturnsfalsefor discrete-time systems with all poles inside the unit circle or with simple poles on it; previously it returnedtruefor every discrete-time system with a nonzero pole.baltruncwith default arguments changes the result only if the largest Hankel singular value is smaller thansqrt(eps())/rtol(about1.5e-5with the defaultrtol), i.e. for systems of small gain, for which it previously removed state variables based on the absolute threshold or threw an error. Users who passedatolexplicitly are not affected.delayd_ss/c2dof delay systems accept delays that are multiples of the sample time up to rounding errors instead of throwing.thiranreturnsz^-Nfor delays that are multiples of the sample time up to rounding errors, instead of a filter of orderN+1.Tests
Regression tests are added to
test_analysis.jl(discrete-timeisunstable: stable, unstable, double pole atz = 0, simple and double poles atz = 1andz = -1, simple and repeated pole pairs on the unit circle, discretized double integrator),test_simplification.jl(baltruncofk*sysfork ∈ {1e-12, 1e-9, 1e-6, 1e6}gives the same state dimension and Hankel singular values scaled byk; zero-gain system) andtest_delayed_systems.jl(delayd_ss(0.3, 0.1),delayd_ss(0.1+0.2, 0.1),c2dof a delay system, rejection ofdelayd_ss(0.31, 0.1),thiran(3*0.1, 0.1) == 1/z^3,thiran(6*0.1, 0.1) == 1/z^6).Run locally (Julia 1.12):
test_analysis.jl,test_simplification.jl,test_matrix_comps.jl,test_delayed_systems.jl,test_discrete.jl: 513 passed, 0 failed, 0 errored.Host:
demeter6, Claude Code sessionsession_01HigNnun4aJKMoWmY7wyULn, working directory~/.julia/dev/ControlSystems/.claude/worktrees/agent-a4840bb7eb32e469c🤖 Generated with Claude Code
https://claude.ai/code/session_01HigNnun4aJKMoWmY7wyULn