Add MATLAB-compatible delay support (InputDelay/OutputDelay/IODelay/InternalDelay)#37
Open
pganguli wants to merge 26 commits into
Open
Add MATLAB-compatible delay support (InputDelay/OutputDelay/IODelay/InternalDelay)#37pganguli wants to merge 26 commits into
pganguli wants to merge 26 commits into
Conversation
Add __adjust_delay__ validation/expansion, store InputDelay/OutputDelay/ IODelay on @LTI with get/set wiring, add hasdelay/totaldelay query helpers, and document the option keys on tf/ss/zpk.
…l/append/feedback Propagate delay correctly through series, parallel, and append; error clearly when feedback operands carry nonzero delay (not yet supported at this point); fix delay-guard handling of numeric operands and 4-arg series found during review.
Add absorbDelay for discrete tf/zpk models, convert InputDelay/ OutputDelay/IODelay between seconds and samples in c2d/d2c, approximate fractional SISO delays via thiran, add the c2dOptions constructor and wire DelayModeling into c2d (renaming 'truncate' to 'state'), absorb delay into extra dynamics for DelayModeling='state', and apply the stored delay's phase factor in freqresp/bode/bodemag/nyquist/nichols/ sigma/sensitivity.
Add a time-response delay-shift helper and apply the stored delay's time shift in step/impulse/ramp/stepinfo/initial and via superposition in lsim.
Add the InternalDelay property and delay-port storage fields on ss; support delay-carrying loops via InternalDelay in feedback/connect, gating delay-port absorption on M participation (fixing a regression where absorption was applied outside the topology feedback/connect actually build) and guarding against multi-output IODelay folding and other non-InternalDelay-aware entry points; evaluate ss InternalDelay via LFT closure in freqresp.
Add InternalDelay support to c2d/d2c via the extended-system trick, simulate InternalDelay systems in step/impulse/ramp/initial/lsim via a per-step delay-buffer lookup, and round ordinary I/O delay too when InternalDelay is also present.
Add guards for InternalDelay in zero(), pole(), and stepinfo() (the last known silent-wrong-answer gap), add MIMO InternalDelay hardening tests across c2d/d2c/step/lsim/freqresp, and fix a regression where the pole() guard broke __sim_horizon__'s auto-horizon estimation for InternalDelay systems.
Extract __internaldelay_ports__ and __delay_lookup__ (previously byte-for-byte duplicated between __time_response__.m and lsim.m) into shared top-level files. Trim a freqresp() wrapper test that re-derived a closed-form already proven by __freqresp__'s own test, and simplify lsim.m's MIMO IODelay test's manual if/else block-shifting into a loop.
__sys_connect_delay__ mis-summed equal-valued IODelay entries in the same column (e.g. two rows both requesting 0.3 would sum to 0.6) and hard-errored via any(sum(iod!=0,1)>1) to avoid returning that wrong value. Replace the summed inport(j) formula with a per-column check over only the genuinely requested (nonzero) entries: if they all agree, use that single value (fixing the equal-value case); if they genuinely differ, keep erroring (rewording the message since this is not a permanent limitation). Also promote three already-working topologies validated during design into permanent regression tests with hand-derived freqresp references: nested feedback around a delayed inner loop, a 3-block delay ring via connect(), and cross-coupled two-block feedback with distinct per-block delays.
A column whose requesting IODelay rows disagree can no longer be folded into one shared series port: a single actuator column b(:,j) cannot carry two different delays to two different outputs. Decompose such a column onto an independent n-state shadow block (x_j' = A x_j + b(:,j) e_j) and tap each output row off it with its own required delay -- a zero-delay row becomes a direct identity feedthrough (no port), a nonzero-delay row an output-side port. One shadow block per decomposed column, shared across its rows. Rows with no genuine open-loop dependence on input j are left untouched. Also fixes the gating bug where the differing-value case was decided before the in_touched(j) check: an IODelay column that M never references now passes straight through unchanged instead of erroring, matching the uniform path's existing gating. Removes the now-obsolete 'not yet supported' hard error. Adds dense, mixed zero/nonzero, and untouched-column regression tests; converts the former error test in feedback.m into a decomposition test.
Adds @lti/pade.m, replacing InputDelay/OutputDelay/IODelay on tf and zpk models with an nth-order Pade rational approximation (ss not yet supported). Order can be a scalar (applied to every nonzero delay) or a vector, one entry per nonzero delay. The per-entry order-vector computation is factored into @lti/__pade_order_vector__.m so it can be reused by the upcoming ss-specific pade dispatches, with the entry ordering convention documented in the helper's header. Guard against querying InternalDelay on tf/zpk models, since that property is ss-specific and get() rejects the key for other classes.
Widen pade.m's ss branch: for ss inputs with hasinternaldelay(sys)==false, convert to tf, defer to the same per-entry substitution loop already used for tf/zpk (now extracted into a shared __pade_substitute_ordinary__ subfunction to avoid duplicating it), then convert back via ss(). This does not preserve the original state basis or state count, matching MATLAB's own pade() semantics. ss inputs with a nonzero InternalDelay still error, now with a message specific to that case (InternalDelay not yet supported) rather than the old blanket "ss not yet supported" -- Task 3 will remove this guard. Adds a cross-check test comparing the ss-roundtrip path against the tf-direct path on the same dense per-entry IODelay system, plus a test for the InternalDelay guard's new message.
Replace the not-yet-supported error for ss models carrying a nonzero InternalDelay with a real approximation: build the extended ordinary plant (__ss_ext_build__), rebuild it as a clean ss so its reported size is correct and all delay-port fields are cleared, then close the delay loop through a diagonal MIMO Padé filter using the lft() Redheffer star product. Ordinary delay carried alongside is handled independently via the existing tf-roundtrip path using the ordinary slice of a single up-front order assignment.
stepinfo only failed for InternalDelay systems because it converted to tf purely to run a per-channel isstable() classification -- everything else it needs (the real step response via __time_response__, and the final value via dcgain, which already routes through __freqresp__'s LFT closure) already worked correctly. Replace the tfdata-based stability check with one run against pade(sys, n) (default order 4, exposed as an optional PadeOrder parameter) for InternalDelay systems only, with a disclosed warning; every other metric keeps using the real, exact time response unchanged. The old guard test used a synthetic fixture (internaldelay set directly, no port matrices -- not producible via feedback()/connect()) that only ever existed to trigger the guard; removing the guard exposed a pre-existing latent bug in dcgain/__time_response__ for that malformed state (empty dcgain, dimension mismatch), out of scope for this fix. Replaced it with a real feedback()-produced fixture.
A nonzero InternalDelay smaller than half the sampling time would silently round to 0 samples, and the delay-buffer simulation's documented tau>=1 assumption (__delay_lookup__/__buffered_sim__) would then drop that port's feedthrough instead of solving the resulting algebraic loop. Convert this from a silent-wrong-answer risk into a clear error, matching every other guard on this branch.
No behavior change -- these still error, just with guidance toward the now-available practical workaround (approximate the delay via pade(), then analyze).
…ternalDelay systems The auto-horizon estimate for InternalDelay systems used only the delay-free rational-part eigenvalues (dssdata's open-loop A matrix), ignoring how the internal-delay port's closed loop can be far more lightly damped than the open-loop dynamics suggest -- a large delay relative to the plant's time constant can push a feedback loop close to marginal stability. For a fixture with a pole at -1 and a 5s delay, the true closed-loop damping is ~40x slower than the open-loop estimate, so step()/impulse()/initial() without an explicit time vector would stop mid-transient instead of showing the settled response. Fix: use pole(pade(sys, 4)) instead for continuous-time InternalDelay systems, capturing the loop's true (approximate) closed-loop pole locations including the delay's effect on damping. pade() only supports continuous-time input (a separate, pre-existing limitation), so an already-discrete InternalDelay system still falls back to the old delay-free eigenvalue estimate. Found while adding auto-horizon test coverage for InternalDelay systems (previously every such test passed an explicit time vector, never exercising this path).
…construction Replaces the Thiran-filter approximation for fractional (non-integer-sample) continuous delay with an exact split-zero-order-hold discretization (Astrom & Wittenmark), for method="zoh"/"std". Matches MATLAB's actual c2d() output exactly: order-preserving denominator, extra numerator term, pure z^-n delay factor -- confirmed against a real MATLAB reference example. New helper inst/__c2d_frac_zoh__.m implements the core math. Non-zoh methods continue to use the existing Thiran approximation, unchanged.
The lti-conversion branch of zpk() discarded these fields entirely -- found while wiring the new exact fractional-delay c2d() path, which is the first code path to call zpk() on a system that still carries a real delay. Unrelated to that fix otherwise; a genuine pre-existing bug.
…lay) Generalizes the SISO exact split-ZOH fix to MIMO, scoped to systems where the fractional delay is attributable to per-input-channel delay: OutputDelay zero-or-uniform across outputs, IODelay uniform down each column. Genuinely row-varying delay still falls back to the existing "not yet supported" error. Also fixes two bugs found while generalizing: a crash converting a SIMO (multi-output, single-input) delayed system to tf/zpk, and a one-sample delay double-counting error affecting any ss-typed SISO fractional-delay output (present since the SISO commit, only caught once ss-typed output was cross-checked against independent ground-truth simulation).
transpose() swapped inname/outname/ingroup/outgroup for the new shape but never touched InputDelay/OutputDelay/IODelay -- every subclass __transpose__ hook (tf, ss, frd; zpk round-trips through tf) leaves these lti-level fields completely untouched, so they stayed shaped/valued for the pre-transpose orientation. Single-point fix in @lti/transpose.m (fixes all four subclasses uniformly): swap InputDelay/OutputDelay and transpose IODelay to match. ctranspose() has no correct fix of the same shape: pertransposition substitutes s -> -s (continuous) or z -> 1/z (discrete), turning a delay into a non-representable negative advance. Errors clearly instead of silently returning stale data, matching the existing InternalDelay guard pattern already used elsewhere in this codebase. Found as a deferred follow-up from the exact-fractional-delay-c2d phase, where the zpk() delay-propagation fix made this newly reachable.
…e continuous path
The dummy validation call in the discrete ordinary-delay path (pade(1, orders(k));) ran with nargout=0, which triggers pade()'s plotting branch -- popping open stray step-response/Bode figure windows on every discrete-delay pade() call. Force nargout=2 to match the continuous path's own call shape and keep the validation call pure.
…losure Closes the discretized InternalDelay loop with an exact z^-k filter per delay port (via __exact_discrete_delay_ss__ + lft(), mirroring pade.m's __pade_substitute_internal__), instead of silently ignoring DelayModeling="state" for InternalDelay systems as before.
Author
|
By the way, I would consider any numerical differences in the output of functions that use the new delay parameters from equivalent MATLAB code to be bugs. Please feel free to let me know if you find any such in-scope differences and I will be happy to take a look into fixing them. Additional discussion may be found in this thread at Octave Discourse |
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.
Brings
pkg-control's time-delay handling up to parity with MATLAB's Control System Toolbox: fullInputDelay/OutputDelay/IODelay/InternalDelaysupport across the toolbox, including exact (not approximate) discretization for the common case.What this adds
InputDelay/OutputDelay/IODelayproperties, correctly composed acrossseries/parallel/append/feedback, and honored inc2d/d2cand time-domain simulation.InternalDelay: an LFT representation for delays trapped inside a feedback loop, with discretization, simulation, and frequency response.c2d()now matches MATLAB exactly forzohon delays that aren't a whole number of sample periods (SISO and a well-defined MIMO case), rather than only approximating.pade()generalized into a full model-level function: approximates any delay (ordinary or internal, continuous or discrete, ontf/zpk/ss) with a rational filter.c2d'sDelayModeling="state"option, including forInternalDelay.transpose()correctly swaps/transposes delay fields;ctranspose()on a delayed system now errors clearly rather than returning a wrong result (a conjugate transpose would need a negative delay, which doesn't exist).Test suite grew substantially: 163 new tests were added. 10 pre-existing skips unrelated to delays remain.
What this doesn't cover yet
InternalDelay.zero(),pole(), andstepinfo()error out on anInternalDelaysystem and point the user atpade()as a workaround (approximate, not exact). MATLAB computes these exactly via a transcendental root-finder; matching that is a substantial piece of work on its own and is the biggest gap left after this PR.dss) models combined with delay are untested. Nothing actively forbids it, but no test exercises this combination, so correctness there is unknown rather than confirmed.connect/feedbackwidening.zoh-only.c2d()matches MATLAB exactly for the defaultzohmethod on fractional-sample delays; thefoh,tustin, andmatchedmethods still fall back to the older approximation for such delays. It's not yet confirmed whether MATLAB itself does anything more exact for those methods, so this may not be a real gap. Worth checking before treating it as follow-up work.ctranspose()on a delayed system is unsupported by design, not an oversight. A conjugate transpose substitutess -> -s(continuous) orz -> 1/z(discrete), which turns a delay into an advance. This is unrepresentable, since delay properties are nonnegative-only. It errors clearly instead of silently returning a wrong answer, and is not planned to change.Illustrative examples
Each snippet below runs unmodified in Octave (with this branch's
pkg-controlloaded) and produces the shown output.