Skip to content

Clamp default frequency grid to Nyquist for discrete diskmargin and mu#148

Merged
baggepinnen merged 1 commit into
masterfrom
fix-diskmargin-discrete-nyquist
May 16, 2026
Merged

Clamp default frequency grid to Nyquist for discrete diskmargin and mu#148
baggepinnen merged 1 commit into
masterfrom
fix-diskmargin-discrete-nyquist

Conversation

@baggepinnen
Copy link
Copy Markdown
Member

Summary

  • sim_diskmargin(L, σ, l, u) and structured_singular_value(M0::LTISystem) (no-w form) both used a default frequency grid up to 1e3 rad/s. For discrete-time inputs whose Nyquist π/Ts is below 1e3, the grid extended past Nyquist and freqresp aliased (cis(ω·Ts) is 2π/Ts-periodic), so the worst-case margin / μ from the default grid reflected an aliased frequency rather than the intended one.
  • Clamp the upper bound to min(u, π/L.Ts) when the system is discrete. The MIMO branch of diskmargin(L, σ; l, u) benefits automatically since it forwards to sim_diskmargin. Continuous-time behavior is unchanged. Users who deliberately want frequencies past Nyquist can still pass an explicit w vector.

Test plan

  • New regression test in test_diskmargin.jl with Ts=1.0 (Nyquist ≈ 3.14 rad/s, well below the default 1e3): asserts the default-grid result agrees with an explicit Nyquist-respecting grid and that the worst-case frequency is at or below Nyquist.
  • Existing fast-sampled discrete test (Ts=1e-3, Nyquist ≈ 3141 > 1e3) is unaffected because the clamp resolves to min(1e3, 3141) = 1e3.
  • Continuous-time paths unchanged.

🤖 Generated with Claude Code

`sim_diskmargin(L, σ, l, u)` and `structured_singular_value(M0::LTISystem)` both
built their default frequency grid up to 1e3 rad/s. For discrete-time inputs
whose Nyquist frequency π/Ts is below 1e3, that grid extended beyond Nyquist
and `freqresp` aliased (`cis(ω·Ts)` is 2π/Ts-periodic), so the worst-case
margin / μ computed on the default grid reflected an aliased frequency rather
than the intended one. The MIMO branch of `diskmargin(L, σ; l, u)` is also
fixed because it forwards to `sim_diskmargin`.

Clamp the upper bound to `min(u, π/L.Ts)` (or `min(3.0, log10(π/M0.Ts))` in
log-frequency) when the system is discrete. Continuous-time behavior is
unchanged. Users who deliberately want frequencies past Nyquist can still pass
an explicit `w` vector.

Adds a regression test in `test/test_diskmargin.jl` using `Ts=1.0` (Nyquist ≈
3.14 rad/s, well below the default 1e3) that confirms the default-grid result
agrees with an explicit Nyquist-respecting grid and that the worst-case
frequency is at or below Nyquist.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.78%. Comparing base (14a7deb) to head (559767e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #148      +/-   ##
==========================================
- Coverage   91.84%   91.78%   -0.06%     
==========================================
  Files          20       20              
  Lines        3064     3066       +2     
==========================================
  Hits         2814     2814              
- Misses        250      252       +2     
Flag Coverage Δ
unittests 91.78% <100.00%> (-0.06%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 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 5f9bafa into master May 16, 2026
2 checks passed
@baggepinnen baggepinnen deleted the fix-diskmargin-discrete-nyquist branch May 16, 2026 04:10
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