Skip to content

evalfr does not behave as expected [sf#6] #22

Description

@murrayrm

*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

  1. changed the title [-]evalfr does not behave as expected[/-] [+]evalfr does not behave as expected [sf#6][/+] on Aug 9, 2014
  2. self-assigned this
    on Aug 9, 2014
  3. murrayrm commented on Aug 9, 2014

    @murrayrm
    MemberAuthor

    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)

  4. murrayrm commented on Aug 9, 2014

    @murrayrm
    MemberAuthor

    Updated by murrayrm on 2014-03-22 18:33 UTC

    • status: open --> accepted
    • assigned_to: Richard Murray
  5. murrayrm commented on Aug 9, 2014

    @murrayrm
    MemberAuthor

    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.

  6. murrayrm commented on Aug 9, 2014

    @murrayrm
    MemberAuthor

    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:

    • status: open --> accepted
    • assigned_to: Richard Murray
    • Comment:

    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?

    Rene: did you already make these changes? If so, I can pull from that
    branch.

    Status: accepted
    Group: Next_Release_(example)
    Created: Fri May 24, 2013 02:32 PM UTC by Anonymous
    Last Updated: Thu May 30, 2013 03:53 PM UTC
    Owner: Richard Murray

    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

    Sent from sourceforge.net because you indicated interest in
    https://sourceforge.net/p/python-control/feature-requests/6/

    To unsubscribe from further messages, please visit
    https://sourceforge.net/auth/subscriptions/

    René van Paassen | o/| Rene.vanPaassen@gmail.com
    <[
    ____-----< t: +31 15 2628685
    | o' mobile: +31 6 39846891

  7. cwrowley commented on Aug 13, 2014

    @cwrowley
    Contributor

    For FRD models, I suggest eliminating evalfr altogether (replacing the existing routine with a different name, such as eval_single_freq). This behavior would be consistent with MATLAB, in which freqresp(sys,omega) works for an FRD model sys, but evalfr(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.

  8. murrayrm commented on Aug 14, 2014

    @murrayrm
    MemberAuthor

    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 evalfr function.

  9. murrayrm commented on Dec 27, 2017

    @murrayrm
    MemberAuthor

    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.

  10. modified the milestones: 0.7.0 , 0.8.0 on Dec 27, 2017
  11. removed their assignment
    on Jan 1, 2018
  12. murrayrm commented on Jan 3, 2018

    @murrayrm
    MemberAuthor

    Another possible approach to handling this:

    • Define the __call__ method in TransferFunction to allow G(s) to return the value of the transfer function G at the complex number s.

    • Remove the evalfr method from TransferFunction, StateSpace, and FRD classes since this functionality is present through __call__.

    • Optionally remove the evalfr function in lti.py. (I'm not sure if this removal is useful since lti.evalfr() currently acts in the same way as the MATLAB evalfr function => no confusion likely.)

    • Define the evalfr function in the MATLAB module to behave in the same way as MATLAB's evalfr function.

    This approach would get rid of the current inconsistency in the code and also avoid anyone who is using {TransferFunction, StateSpace, FRD}.evalfr from having a bug suddenly appear due to a change in the way evalfr is handled. The rationale for removing the evalfr function in lti.py is to encourage the use of of G(s) for transfer functions (seems more elegant) or sys.freqresp(omega) for any LTI system.

  13. cwrowley commented on Jan 3, 2018

    @cwrowley
    Contributor

    I like all of these ideas: using the __call__ method for TransferFunction objects, removing the evalfr methods, and moving the evalfr function to the MATLAB module. Though it may be more polite to keep the evalfr routines for a little while and give a deprecation warning, just so anybody using this doesn't find their code suddenly broken without warning.

  14. self-assigned this
    on Jan 4, 2018
  15. murrayrm commented on Jan 6, 2018

    @murrayrm
    MemberAuthor

    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 evalfr method functionality, but renamed it to _evalfr, since it was being used by margins.py and frdata.py in what seemed like sensible ways. See PR #179 for more details on the changes.

  16. murrayrm commented on Jan 13, 2018

    @murrayrm
    MemberAuthor

    Addressed in PR #179

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions