- Notifications
You must be signed in to change notification settings - Fork445
Disk margin calculations#1146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Disk margin calculations#1146
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…andler comment, fix typo in skew description of disk_margins docstring
…e function to plot allowable gain/phase variations.
…n the margins module
calculation of 'f', the bounding complex curve. Seems to look correct for balanced(skew = 0) case, still verifying the skewed equivalent.
…on-control into jdelange/disk-margins
…ol/tests/margin_test.py
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
josiahdelange commentedApr 26, 2025
I fixed the docstrings and implemented the linter's recommendations so that automated workflows finally pass. One thing I previously hadn't appreciated was the complexity of the control plotting code, so to make this PR simpler I'm only proposing to add a library function to calculate the disk-based margins. All other plotting is left to user code (or future PRs), e.g. the local function |
coveralls commentedApr 28, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
murrayrm left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Some additional suggestions attached, mainly focused on coding style (to be consistent with the standard style in the package).
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
…reference on disk/ellipse-based margin calculations.
josiahdelange commentedApr 29, 2025
@murrayrm (and@slivingston) thanks for taking time to review. Ithink everything's addressed - but let me know of anything else! |
murrayrm left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Found a few more style issues.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
josiahdelange commentedJun 25, 2025
@murrayrm thanks and latest formatting issues are now fixed. The one failing check "Slycot from source / build-linux (pull_request)" does not seem to be the disk margin tests. |
murrayrm commentedJun 25, 2025
Agreed. Tracking the failed unit test in#1161. I'll go ahead and merge this since the fail check is unrelated. |
aa92b65 intopython-control:mainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Am relatively new to this toolbox, have been using it here and there and found#726.
This is an initial prototype Python implementation of disk margins, built on top of$\mu$ at each discrete frequency. The function
python-controlandslycot. The code should work both for SISO and MIMO systems, the latter of which requiresslycot(for SLICOT'sAB13MD) to compute the upper bound ofdisk_marginscomputes disk margins (and corresponding disk-based gain/phase margins), optionally returning the whole frequency-dependent vectors for further plotting.It's been verified against the example SISO loop transfer functions in thepublished paper and the "spinning satellite" MIMOexample from the MathWorks documentation. I also confirmed SISO disk margins match the relevant output of existing function
stability_marginscorresponding to the Nyquist plot's distance to -1. All seem to match within a few significant digits, so the general behavior seems correct. Might be good to get another set of eyes to double check/test further.I tried to base as much as possible (e.g. style conventions) on existing code in
control/margins.py. Example usage (seeexamples/disk_margin.py):Output:
The example script also shows how to plot the allowable/stable region of gain and phase variations which will not destabilize the loop, in a local function
plot_allowable_region, e.g.