Repository navigation
Fix discrete-time frequency handling in describing_function_response - #1272
Closed
gimballock wants to merge 1 commit into
Closed
gimballock wants to merge 1 commit into
gimballock wants to merge 1 commit into
Conversation
For a discrete-time system, describing_function_response found limit cycles with about the right amplitude but reported the wrong frequency: it took the oscillation frequency as the imaginary part of the Nyquist contour, which for a discrete system lies on the unit circle exp(1j*w*dt), so it reported about sin(w*dt) - the same number whatever dt is. The refinement cost function and the returned positions also evaluated H at z = 1j*w rather than on the unit circle, and refinement typically failed with "not able to refine result; returning estimate". For example, H(z) = 0.5/(z^2 - 0.5z) with dt = 1 crosses the negative real axis at w = 1.3181, but the reported frequency was 0.9703, and dt = 0.5 gave the same 0.9703 instead of 2.6362. When H.isdtime(strict=True) - matching the timebase semantics used by nyquist_response - take the frequencies as np.angle(contour) / dt and evaluate H at exp(1j*w*dt) in the refinement cost function and for the returned positions. The strict check keeps dt=None (unspecified timebase) on the continuous-time path; dt=True (discrete, unspecified sample time) reports rad/sample. Adds regression tests: a known unit-circle crossing for dt = 1 and dt = 0.5, plus dt=None continuous-equivalence and dt=True coverage. The discrete tests fail at baseline; continuous-time behavior is unchanged. Fixes python-control#1269
Member
|
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.
For a discrete-time system,
describing_function_responsefound limit cycles with about the right amplitude but reported the wrong frequency: it took the oscillation frequency as the imaginary part of the Nyquist contour, which for a discrete system lies on the unit circleexp(1j*w*dt), so it reported aboutsin(w*dt)- the same number whatever dt is. The refinement cost function and the returned positions also evaluated H atz = 1j*wrather than on the unit circle, and refinement typically failed with "not able to refine result; returning estimate".For example,
H(z) = 0.5/(z^2 - 0.5z)withdt = 1crosses the negative real axis atw = 1.3181, but the reported frequency was 0.9703, anddt = 0.5gave the same 0.9703 instead of 2.6362.When
H.isdtime(strict=True)- matching the timebase semantics used by nyquist_response - take the frequencies asnp.angle(contour) / dtand evaluate H atexp(1j*w*dt)in the refinement cost function and for the returned positions. The strict check keepsdt=None(unspecified timebase) on the continuous-time path;dt=True(discrete, unspecified sample time) reports rad/sample.Adds regression tests: a known unit-circle crossing for
dt = 1anddt = 0.5, plusdt=Nonecontinuous-equivalence anddt=Truecoverage. The discrete tests fail at baseline; continuous-time behavior is unchanged.Fixes #1269
written by claude opus 5.5, reviewed by me