Repository navigation
sys.dcgain() should return a real value rather than a complex value (until/unless we support complex coefficients) #578
Description
Activity
I agree, I don't know what a complex value mean.
right now
sys.dcgain()returns a number that is always something like10 + 0j. The0jis extraneous (and makes it a complex number rather than a real number) because, at least currently in python-control, the coefficients of our systems are always real, so the dc gain is a real number.We still have #484 open. Support for complex StateSpace matrices was requested by users more than once.
Converting the complex value to real in time response tools like step_info like it is right now, makes sense though.
This happens because
dcgain(sys)is justsys(0)and this is a complex number (this came from PR #542). A quick fix would be to just returnabs(sys(0)).Just to check: if we make this change, the DC gain of a system with negative zero frequency gain would be positive (right now the DC gain for the transfer function 1/(s-1) is -1; with this change it would be +1).
re complex-valued systems: presumably we would update
dcgain()for such systems once they are supported, to allow for complex values?re negative-gain systems: seems like there are two options:
abs(sys(0))orsys(0).real. Matlab seems to use the second, giving-1fordcgain(tf(-1, 1)).my current take: we should use
abs()in the method, and use.realin the matlab layer.But why disallow negative gain?
How about only casting to real if the imaginary part is 0?
return retval.real if np.all(np.isreal(retval)) else retval
https://numpy.org/doc/stable/reference/generated/numpy.isreal.html
Originally I was thinking that dcgain should correspond to the magnitude at zero. But on further thought, gain does not necessarily mean magnitude. so now I like
sys(0).realbetter.I like @bnavigator's idea.
Question: Implement based on current master right now and rebase #577 or merge that first? step_info builds heavily on on
dcgain()My vote is first option. happy to approve a PR on your suggested DCgain modification.
I'm OK with the proposed approach, but I wonder if we shouldn't also (separately) start thinking about making python-control more pythonic.
Why do we need
dcgain(sys)? Isn'tsys(0).realas easy (and the same number of characters, even)? And it is more clear because the meaning is not "hidden" behind an anachronistic term (dc = direct current...).Similarly, why do we have
freqresp(sys, omega)when we can just saysys(1j*omega)?So we could just get rid of some of these extraneous functions and put them all in either
control.matlabor create acontrol.matlab-aliasesthat defines the "standard" MATLAB commands as lightweight functions that call the more pythonic signatures.Reacted by Ben GreinerAlso, I don't have MATLAB on my system any more (cutting the cord!), but what does MATLAB return for
dcgain(tf([1], [1, 0])?Also, I don't have MATLAB on my system any more (cutting the cord!), but what does MATLAB return for
dcgain(tf([1], [1, 0])?>> dcgain(tf([1],[1, 0]) dcgain(tf([1],[1, 0]) ↑ Error: Invalid expression. When calling a function or indexing a variable, use parentheses. Otherwise, check for mismatched delimiters. Did you mean: >> dcgain(tf([1],[1, 0])) ans = Inf
😜
Similarly, why do we have
freqresp(sys, omega)when we can just saysys(1j*omega)One possible reason: if the system is discrete-time, it's cumbersome to type
sys(np.exp(1j*omega*sys.dt))compared tosys.frequency_response(omega).That said, along those lines of simplifying things, I could be convinced that
frequency_responseshould return just the complex response of the system at the frequenciesomega(after a warning if going above the nyquist frequency). Rather than mag, phase, and (sorted) omega as it does now.Good point on
frequency_responsefor discrete time, though we could still get rid offreqrespas a standalone function (keeping it in the MATLAB compatibility module).Did we simply forget to deprecate
lti.freqresp, when we deprecated the.freqresp()methods for the LTI systems?Lti.freqresp is where the matlab layer’s freqresp function is. That’s pretty common to have things all over, not just in matlab.wrappers (eg, same for tf and ss).
