Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 36 commits into
base: main
Choose a base branch
from

Conversation

josiahdelange
Copy link

@josiahdelange josiahdelange commented Apr 24, 2025

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 python-control and slycot. The code should work both for SISO and MIMO systems, the latter of which requires slycot (for SLICOT's AB13MD) to compute the upper bound of $\mu$ at each discrete frequency. The function disk_margins computes 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 the published paper and the "spinning satellite" MIMO example from the MathWorks documentation. I also confirmed SISO disk margins match the relevant output of existing function stability_margins corresponding 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 (see examples/disk_margin.py):

import os, sys, math
import numpy as np
import control

import math
import matplotlib
import matplotlib.pyplot as plt
from warnings import warn

import numpy as np
import scipy as sp

# Frequencies of interest
omega = np.logspace(-1, 3, 1001)

# Laplace variable
s = control.tf('s')

# MIMO loop transfer gain for the spinning satellite example
# https://www.mathworks.com/help/robust/ug/mimo-stability-margins-for-spinning-satellite.html
P = control.ss([[0, 10],[-10, 0]], np.eye(2), [[1, 10], [-10, 1]], [[0, 0],[0, 0]]) # plant
C = control.ss([],[],[], [[1, -2], [0, 1]]) # controller
L = P*C # output loop gain

print(f"------------- Sensitivity function (S) -------------")
DM, GM, PM = control.disk_margins(L, omega, skew = 1.0, returnall = True) # S-based (S)
print(f"min(DM) = {min(DM)} (omega = {omega[np.argmin(DM)]})")
print(f"GM = {GM[np.argmin(DM)]} dB")
print(f"PM = {PM[np.argmin(DM)]} deg")
print(f"min(GM) = {min(GM)} dB")
print(f"min(PM) = {min(PM)} deg\n")

plt.figure(3)
plt.subplot(3,3,1)
plt.semilogx(omega, DM, label='$\\alpha$')
plt.legend()
plt.title('Disk Margin')
plt.grid()
plt.xlim([omega[0], omega[-1]])
plt.ylim([0, 2])

plt.figure(3)
plt.subplot(3,3,4)
plt.semilogx(omega, GM, label='$\\gamma_{m}$')
plt.ylabel('Gain Margin (dB)')
plt.legend()
plt.title('Gain-Only Margin')
plt.grid()
plt.xlim([omega[0], omega[-1]])
plt.ylim([0, 40])

plt.figure(3)
plt.subplot(3,3,7)
plt.semilogx(omega, PM, label='$\\phi_{m}$')
plt.ylabel('Phase Margin (deg)')
plt.legend()
plt.title('Phase-Only Margin')
plt.grid()
plt.xlim([omega[0], omega[-1]])
plt.ylim([0, 90])

print(f"------------- Complementary sensitivity function (T) -------------")
DM, GM, PM = control.disk_margins(L, omega, skew = -1.0, returnall = True) # T-based (T)
print(f"min(DM) = {min(DM)} (omega = {omega[np.argmin(DM)]})")
print(f"GM = {GM[np.argmin(DM)]} dB")
print(f"PM = {PM[np.argmin(DM)]} deg")
print(f"min(GM) = {min(GM)} dB")
print(f"min(PM) = {min(PM)} deg\n")

plt.figure(3)
plt.subplot(3,3,2)
plt.semilogx(omega, DM, label='$\\alpha$')
plt.legend()
plt.title('Disk Margin')
plt.grid()
plt.xlim([omega[0], omega[-1]])
plt.ylim([0, 2])

plt.figure(3)
plt.subplot(3,3,5)
plt.semilogx(omega, GM, label='$\\gamma_{m}$')
plt.ylabel('Gain Margin (dB)')
plt.legend()
plt.title('Gain-Only Margin')
plt.grid()
plt.xlim([omega[0], omega[-1]])
plt.ylim([0, 40])

plt.figure(3)
plt.subplot(3,3,8)
plt.semilogx(omega, PM, label='$\\phi_{m}$')
plt.ylabel('Phase Margin (deg)')
plt.legend()
plt.title('Phase-Only Margin')
plt.grid()
plt.xlim([omega[0], omega[-1]])
plt.ylim([0, 90])

print(f"------------- Balanced sensitivity function (S - T) -------------")
DM, GM, PM = control.disk_margins(L, omega, skew = 0.0, returnall = True) # balanced (S - T)
print(f"min(DM) = {min(DM)} (omega = {omega[np.argmin(DM)]})")
print(f"GM = {GM[np.argmin(DM)]} dB")
print(f"PM = {PM[np.argmin(DM)]} deg")
print(f"min(GM) = {min(GM)} dB")
print(f"min(PM) = {min(PM)} deg\n")

plt.figure(3)
plt.subplot(3,3,3)
plt.semilogx(omega, DM, label='$\\alpha$')
plt.legend()
plt.title('Disk Margin')
plt.grid()
plt.xlim([omega[0], omega[-1]])
plt.ylim([0, 2])

plt.figure(3)
plt.subplot(3,3,6)
plt.semilogx(omega, GM, label='$\\gamma_{m}$')
plt.ylabel('Gain Margin (dB)')
plt.legend()
plt.title('Gain-Only Margin')
plt.grid()
plt.xlim([omega[0], omega[-1]])
plt.ylim([0, 40])

plt.figure(3)
plt.subplot(3,3,9)
plt.semilogx(omega, PM, label='$\\phi_{m}$')
plt.ylabel('Phase Margin (deg)')
plt.legend()
plt.title('Phase-Only Margin')
plt.grid()
plt.xlim([omega[0], omega[-1]])
plt.ylim([0, 90])

Output:

------------- Sensitivity function (S) -------------
min(DM) = 0.3697398698410271 (omega = 0.1)
GM = 2.732761944304522 dB
PM = 21.30709883385843 deg
min(GM) = 2.732761944304522 dB
min(PM) = 21.30709883385843 deg

------------- Complementary sensitivity function (T) -------------
min(DM) = 0.3683035923739884 (omega = 0.1)
GM = 2.7236493433653735 dB
PM = 21.223368586091564 deg
min(GM) = 2.7236493433653735 dB
min(PM) = 21.223368586091564 deg

------------- Balanced sensitivity function (S - T) -------------
min(DM) = 0.3769872636944355 (omega = 0.1)
GM = 3.3140985364257256 dB
PM = 21.349285542574695 deg
min(GM) = 3.3140985364257256 dB
min(PM) = 21.349285542574695 deg

Figure_3

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.

.
.
.
DM_plot = []
DM_plot.append(control.disk_margins(L, omega, skew = -1.0)[0]) # T-based (T)
DM_plot.append(control.disk_margins(L, omega, skew = 0.0)[0]) # balanced (S - T)
DM_plot.append(control.disk_margins(L, omega, skew = 1.0)[0]) # S-based (S)
plt.figure(30)
plot_allowable_region(DM_plot, skew = [-1.0, 0.0, 1.0])

Figure_30

…andler comment, fix typo in skew description of disk_margins docstring
…e function to plot allowable gain/phase variations.
calculation of 'f', the bounding complex curve.  Seems to look correct for balanced
(skew = 0) case, still verifying the skewed equivalent.
Comment on lines 24 to 33
try:
from . import mag2db
except ImportError:
# Likely due the following circular import issue:
#
# ImportError: cannot import name 'mag2db' from partially initialized module
# 'control' (most likely due to a circular import) (control/__init__.py)
#
def mag2db(mag):
return 20*np.log10(mag)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to import from the module where this is defined:

from .ctrlutil import mag2db

>>
>> omega = np.logspace(-1, 3, 1001)
>>
>> P = control.ss([[0, 10],[-10, 0]], np.eye(2), [[1, 10], [-10, 1]], [[0, 0],[0, 0]])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example seems too big for a docstring, and anyway, it already is given in examples/disk_margins.py. Is there a smaller example that can be given in this docstring?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree. Yes - will remove all the plotting.

@josiahdelange josiahdelange marked this pull request as draft April 26, 2025 14:29
@josiahdelange
Copy link
Author

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 plot_allowable_region in examples/disk_margins.py.

@josiahdelange josiahdelange marked this pull request as ready for review April 26, 2025 16:03
@coveralls
Copy link

coveralls commented Apr 28, 2025

Coverage Status

coverage: 94.732% (-0.01%) from 94.746%
when pulling fe79760 on josiahdelange:jdelange/disk-margins
into 632391c on python-control:main.

Copy link
Member

@murrayrm murrayrm left a 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).

Comment on lines 413 to 421
assert_allclose([DMo], [0.3754], atol = 0.1) # disk margin of 0.3754
assert_allclose([DGMo], [3.3], atol = 0.1) # disk-based gain margin of 3.3 dB
assert_allclose([DPMo], [21.26], atol = 0.1) # disk-based phase margin of 21.26 deg

# Balanced (S - T) disk-based stability margins at plant input
DMi, DGMi, DPMi = disk_margins(Li, omega, skew = 0.0)
assert_allclose([DMi], [0.3754], atol = 0.1) # disk margin of 0.3754
assert_allclose([DGMi], [3.3], atol = 0.1) # disk-based gain margin of 3.3 dB
assert_allclose([DPMi], [21.26], atol = 0.1) # disk-based phase margin of 21.26 deg
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code never runs if slycot is not loaded => can you just remove it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - will fix.

Lo = P*K # loop transfer function, broken at plant output
Li = K*P # loop transfer function, broken at plant input

if importlib.util.find_spec('slycot') == None:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can just use ct.slycot_check() here and avoid using the importlib call.

@@ -517,3 +521,130 @@ def margin(*args):
% len(args))

return margin[0], margin[1], margin[3], margin[4]

def disk_margins(L, omega, skew = 0.0, returnall = False):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove spaces around = to be consistent with python-control (PEP8) coding conventions.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix

Comment on lines 538 to 540
skew = 0 uses the "balanced" sensitivity function 0.5*(S - T)
skew = 1 uses the sensitivity function S
skew = -1 uses the complementary sensitivity function T
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which of these is the default value?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will update. Default of 0.0 is consistent with the MATLAB function diskmargin.

skew = 1 uses the sensitivity function S
skew = -1 uses the complementary sensitivity function T
returnall : bool, optional
If true, return frequency-dependent margins. If False (default),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true → True

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix

DM = np.zeros(omega.shape, np.float64)
DGM = np.zeros(omega.shape, np.float64)
DPM = np.zeros(omega.shape, np.float64)
for ii in range(0,len(omega)):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space after comma (PEP8).

DPM = np.zeros(omega.shape, np.float64)
for ii in range(0,len(omega)):
# Disk margin (a.k.a. "alpha") vs. frequency
if L.issiso() and (ab13md == None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parentheses around second expression are not needed.

np.array(num_loops*[2]))[0]

# Disk-based gain margin (dB) and phase margin (deg)
with np.errstate(divide = 'ignore', invalid = 'ignore'):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove excess spaces around keyword assignment operator.


if returnall:
# Frequency-dependent disk margin, gain margin and phase margin
return (DM, DGM, DPM)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parentheses not needed here.

Comment on lines 648 to 650
return ((not DM.shape[0] and float('inf')) or np.amin(DM),
(not gmidx != -1 and float('inf')) or DGM[gmidx][0],
(not DPM.shape[0] and float('inf')) or DPM[pmidx][0])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found this very hard to parse. I wonder whether using if-else syntax might be simpler:

return (
    float('inf') if DM.shape[0] == 0 else np.amin(DM),
    float('inf') if gmidx == -1 else DGM[gmidx][0],
    float('inf') if DPM.shape[0] == 0 else DPM[pmidx][0])

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree - thanks for the snippet, that is much cleaner. I tried to emulate stability_margins, but had whittled it down somewhat. Will update.

@josiahdelange
Copy link
Author

@murrayrm (and @slivingston) thanks for taking time to review. I think everything's addressed - but let me know of anything else!

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.

4 participants