Repository navigation
evalfr does not behave as expected [sf#6] #22
Description
*Reported by anonymous on 2013-05-24 14:32 UTC
The current implementation of evalfr is different from the matlab function evalfr. Could you please "fix" it.
Example: Evaluate the transfer function G=tf(1,[2,3,4]) at s=1+2j
In Matlab one would type evalfr(G,s) and get the expectet result (0.0051-0.0711j).
Using this module typing evalfr(G,s) will actually compute evalfr(G,s*j) which is (0.09756098+0.12195122j), that is, it evaluated G at the point -2+j instead.
I agree that this is what one would expect (after reading the documentation of evalfr) but I would rather have a behavior that conforms with matlab (after all the module control.matlab was designed to emulate matlab's routines)
Thanks,
Juan C. Cockburn
juan.cockburn@rit.edu
Activity
[-]evalfr does not behave as expected[/-][+]evalfr does not behave as expected [sf#6][/+]murrayrm commentedon Aug 9, 2014
Commented by repa on 2013-05-30 15:53 UTC
I fixed this in my branch (mix-matrices-and-sys)
Added two checks in matlab_test.py, one with siso and one with mimo, which gives the same puzzling results as Matlab (basically, both inputs same, with only a scalar complex number as input)
murrayrm commentedon Aug 9, 2014
Updated by murrayrm on 2014-03-22 18:33 UTC
murrayrm commentedon Aug 9, 2014
Commented by murrayrm on 2014-03-22 18:33 UTC
I propose that we change the behavior of evalfr() in python-control to match that of MATLAB. Namely, the second argument should be a complex number. If you want to evaluate at frequency omega, use sys.evalfr(omega*1j).
This will require changing evalfr in ss, tf, FRD and other locations. Any objections?
Note: currently, the matlab.evalfr() function behaves like MATLAB, but sys.evalfr() takes a real valued frequency as its argument. The proposed change would flip this so that sys.evalfr() takes a complex number and returns the frequency response at that complex number.
murrayrm commentedon Aug 9, 2014
Commented by repa on 2014-03-23 12:03 UTC
No, I didn't modify that function.
Do you want to change only the Matlab one, or also the ones in the transfer
function etc? Simply multiplying the argument by -j should do it then
On 22 March 2014 19:33, Richard Murray murrayrm@users.sf.net wrote:
René van Paassen | o/| Rene.vanPaassen@gmail.com
<[____-----< t: +31 15 2628685
| o' mobile: +31 6 39846891
cwrowley commentedon Aug 13, 2014
For FRD models, I suggest eliminating
evalfraltogether (replacing the existing routine with a different name, such aseval_single_freq). This behavior would be consistent with MATLAB, in whichfreqresp(sys,omega)works for an FRD modelsys, butevalfr(sys,1i*omega)gives an error.Since FRD models cannot be evaluated off the imaginary axis, the alternative seems to be to have inconsistent behavior between FRD models and tf/ss models, which would be bad.
murrayrm commentedon Aug 14, 2014
Definitely agree that evaluating an FRD model off the imaginary axis should generate an error, but I think it is OK if it is evaluated on a pure imaginary number. Not a major issue if it doesn't match the MATLAB
evalfrfunction.murrayrm commentedon Dec 27, 2017
In looking through the code, it appears that this issue is still open and not yet addressed. Updating milestone to 0.8.0 since it should be fixed for that release.
murrayrm commentedon Jan 3, 2018
Another possible approach to handling this:
Define the
__call__method inTransferFunctionto allowG(s)to return the value of the transfer functionGat the complex numbers.Remove the
evalfrmethod fromTransferFunction,StateSpace, andFRDclasses since this functionality is present through__call__.Optionally remove the
evalfrfunction inlti.py. (I'm not sure if this removal is useful sincelti.evalfr()currently acts in the same way as the MATLABevalfrfunction => no confusion likely.)Define the
evalfrfunction in the MATLAB module to behave in the same way as MATLAB'sevalfrfunction.This approach would get rid of the current inconsistency in the code and also avoid anyone who is using
{TransferFunction, StateSpace, FRD}.evalfrfrom having a bug suddenly appear due to a change in the wayevalfris handled. The rationale for removing theevalfrfunction inlti.pyis to encourage the use of ofG(s)for transfer functions (seems more elegant) orsys.freqresp(omega)for any LTI system.cwrowley commentedon Jan 3, 2018
I like all of these ideas: using the
__call__method forTransferFunctionobjects, removing theevalfrmethods, and moving theevalfrfunction to the MATLAB module. Though it may be more polite to keep theevalfrroutines for a little while and give a deprecation warning, just so anybody using this doesn't find their code suddenly broken without warning.murrayrm commentedon Jan 6, 2018
The fixes described above have (roughly) been implemented in PR #179. The main difference from what we discussed is that I left in the old
evalfrmethod functionality, but renamed it to_evalfr, since it was being used bymargins.pyandfrdata.pyin what seemed like sensible ways. See PR #179 for more details on the changes.murrayrm commentedon Jan 13, 2018
Addressed in PR #179