镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Use relative root distances for transfer function minreal - #1089

Merged
baggepinnen merged 3 commits into
JuliaControl:masterfrom
dongxuelian2:codex/fix-minreal-relative-tolerance
Oct 22, 2026
Merged

baggepinnen merged 3 commits into
JuliaControl:masterfrom
dongxuelian2:codex/fix-minreal-relative-tolerance

Conversation

@dongxuelian2

@dongxuelian2 dongxuelian2 commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Part of ControlSystems.jl#1081: transfer-function pole-zero cancellation, including the implicit cancellation in ZPK addition.

The fixed sqrt(eps(Float64)) absolute distance can remove distinct slow dynamics and miss common factors at large scales or in Float32. This changes the default to the local relative distance abs(p-z)/max(abs(p),abs(z)) < sqrt(eps(T)), with T taken from the roots. Equal roots at zero still cancel. There is no global magnitude floor: an unrelated fast root must not make distinct slow roots eligible for cancellation. A common root scaling leaves the criterion unchanged, apart from rounding.

Explicit minreal(G, tol) keeps its existing absolute-distance meaning and strict comparison. Conjugate-pair adjustments are restricted to real-coefficient models. Complex-coefficient models cancel only the matched pole and zero, preserving all remaining roots under both relative and explicit absolute tolerances. No dependencies or state-space algorithms change.

The boundary-mode conversion test now passes its existing absolute tolerance to both reductions: a numerically shifted integrator does not match an exact zero under the conservative relative default. Its response assertion is unchanged.

Example Before After
minreal(tf([1,1.5e-8],[1,1e-8])): DC gain 1 1.5
zpk([],[-1e-8],1e-8) + 1: DC gain 1 2
f+f, f=zpk([-2e6],[-1e6,-3e6,-4e6],1e12): stored poles 5 3

This remains a root-matching heuristic, not a response-error bound. In particular, proximity to the imaginary axis/unit circle and ill-conditioned repeated polynomial roots require care. These limitations and the tolerance semantics are documented. Large roots can now match when relatively close even if their absolute distance exceeded the old threshold.

Validation (Windows x86_64):

  • The unmodified upstream baseline fails both new assertions for the reported DC gain example.
  • New regressions: 613 assertions passed, including independent BigFloat coefficient evaluations.
  • Original ZPK, transfer-function, complex and simplification tests: 269 passed, 2 existing broken tests.
  • Fixed-seed standalone adversarial experiments: 8,800 assertions passed, including independent 256-bit evaluations of original/reduced responses and rounded polynomial coefficients.
  • Full ControlSystems tests: 1,156 passed on Julia 1.10.12.
  • Full ControlSystemsBase tests on Julia 1.13.2: 6,957 passed, 6 existing broken tests; Aqua passes.

The initial Julia 1.10 Base run exposed the conversion-test assumption corrected above and a pre-existing state-space freqresp allocation failure. The allocation reproducer is identical on unmodified master and this branch (123,472 bytes); Julia 1.13 allocates 19,184 bytes and passes the original threshold. The corrected conversion cases also pass on Julia 1.10.

Reproduce the new regression file with:

julia --project=lib/ControlSystemsBase -e 'using ControlSystemsBase, Test, Random; include("lib/ControlSystemsBase/test/test_tf_minreal.jl")'

AI-assisted implementation, numerical experiments, and review with Codex.

Maintainer follow-up validation (demeter2, Julia 1.13.1):

  • Added 139 assertions for complex-coefficient cancellation under relative and explicit absolute tolerances, both root orderings, preservation of unmatched poles and zeros, source-model immutability, real-coefficient conjugate handling, ZPK addition, and rational transfer-function conversion. Before the fix, 38 of these assertions failed.
  • The tolerance, ZPK, transfer-function, complex-system, simplification, and conversion suites pass: 1,095 assertions, with 2 existing broken tests.

Maintainer update: host demeter2; Codex session 01a15eb9-1234-7a03-a4be-7f8814d35ca2; working directory /tmp/controlsystems-review-1089.

@codecov

codecov Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.26%. Comparing base (594c0eb) to head (9e83410).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1089      +/-   ##
==========================================
+ Coverage   92.22%   92.26%   +0.04%     
==========================================
  Files          42       42              
  Lines        6146     6170      +24     
==========================================
+ Hits         5668     5693      +25     
+ Misses        478      477       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@baggepinnen
baggepinnen merged commit 9baa912 into JuliaControl:master Oct 22, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants