bugGNU Octave - Bugs: bug #43895, freqz plot should extend to end of...

 
 

bug #43895: freqz plot should extend to end of range, not 1 data point short.

Submitter:  None
Submitted:  Sat 27 Dec 2014 01:40:21 AM UTC
   
 
Category:  Octave Function Severity:  3 - Normal
Priority:  5 - Normal Item Group:  None
Status:  Fixed Assigned to:  None
Originator Name:  Bill McEachen Originator Email:  -email is unavailable-
Open/Closed:  * Closed Release:  * 3.6.4
Operating System:  * Microsoft Windows Fixed Release:  None
Planned Release:  None
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Tue 10 Feb 2015 08:51:16 PM UTC, comment #15: 

You're welcome, and thank you for taking time to write a note. 

There are no paid staff for Octave and everyone of us here is a volunteer.  But most of the time it's all silence when we solve a problem.

Rik <rik5>
Group administrator
Tue 10 Feb 2015 07:35:02 PM UTC, comment #14: 

as the original reporter, I VERY much appreciate all of the efforts.

Anonymous
Tue 10 Feb 2015 05:23:04 PM UTC, comment #13: 

@Doug: I added some input validation and committed your changeset here (http://hg.savannah.gnu.org/hgweb/octave/rev/8724af455b11).  Thanks, the plots look right to my eye now.  Closing report.

Rik <rik5>
Group administrator
Mon 09 Feb 2015 09:28:10 PM UTC, comment #12: 

I have attached a patch that I think does what we want.
please try it.

(file #33040)

Doug Stewart <dastew>
Wed 04 Feb 2015 04:18:12 PM UTC, comment #11: 

Thanks, Mike.

I'm not sure why the graph "should" go to the pi or 2*pi value, given that the Matlab graph doesn't.  However, I agree that it should plot all 10 points...

Sorry to have added only noise to this discussion :(

Lachlan Andrew <lachlan>
Wed 04 Feb 2015 01:50:30 PM UTC, comment #10: 

You didn't paste the results of getting the x and y data from the plot, but someone on the list already provided that. Including here for completeness in case this gets forgotten:


>> get(h,'xdata')

ans =

         0    0.2000    0.4000    0.6000    0.8000    1.0000 1.2000    1.4000    1.6000    1.8000    2.0000

>> get(h,'ydata')

ans =

   -2.2789    0.3236   14.0514   -0.6138   -5.8441   -7.2346   -5.8441   -0.6138   14.0514    0.3236       Inf


So this is one more point than what is returned, and one more point than what Octave currently plots. So I think we all agree that the return value is correctly 10 points, but the plot should extend to the pi or 2*pi value.

Mike Miller <mtmiller>
Group Member
Wed 04 Feb 2015 06:55:13 AM UTC, comment #9: 

Rik, the output of your sequence of commands is attached.  The last point is 1.8

(file #32990)

Lachlan Andrew <lachlan>
Fri 30 Jan 2015 05:31:39 PM UTC, comment #8: 

@Doug: Maybe you can ask on the octave-maintainers list for someone to run


freqz (1, [1, -0.5, 0.8], 10, 'whole');
subplot (2,1,1);
h = get (gca, 'children');
get (h, 'xdata')


and see what the last x data point is.

Rik <rik5>
Group administrator
Fri 30 Jan 2015 05:21:07 PM UTC, comment #7: 

After looking at this for some time, I think this patch is what we want.
This adds 1 more point to the plot only for the half range. I dont think we need to change the "whole" plot, as the next point will be the same as the first point, and it is the DC (0 freq.) value.


(file #32951)

Doug Stewart <dastew>
Mon 12 Jan 2015 12:52:00 AM UTC, comment #6: 

Could somebody with access to Matlab run the following and report the results.


[a,b] = freqz (1, [1, -0.5, 0.8], 10)


@Doug: using your patch this returns an 11-point vector whereas the documentation seems to imply this should return a 10-point vector (same as n).  I think instead of changing the value of 'n' we want to change the spacing of Fs.

The code change below works to return a 10-point vector and display everything out to a normalized frequency of 1.0.


-    f = Fs * (0:n-1).' / N;
+    f = Fs * linspace (0, n, n).' / N;




Rik <rik5>
Group administrator
Mon 29 Dec 2014 05:52:31 PM UTC, comment #5: 

    if (strcmp (region, "whole"))
      N = n;
    else
      N = 2*n;
    endif
    n=n+1; # add 1 to get all the way to PI
    f = Fs * (0:n-1).' / N;

    pad_sz = N*ceil (k/N);
    b = postpad (b, pad_sz);
    a = postpad (a, pad_sz);


This is where I put it.
I did not study the code to see if this is the best fix, just a quick try.

Doug Stewart <dastew>
Mon 29 Dec 2014 05:19:57 PM UTC, comment #4: 

Confirmed.  It does seem like Octave should plot right up to either pi or 2*pi if 'whole' is given.

@Doug: Line 155 is a blank line in my copy of freqz.m on the development branch.  Could you post the result of 'diff -u' so I can see where you were adding n=n+1?

Rik <rik5>
Group administrator
Sat 27 Dec 2014 02:27:45 AM UTC, comment #3: 

Try this at line 155:

 n=n+1; # add 1 to get all the way to PI

Doug Stewart <dastew>
Sat 27 Dec 2014 02:17:41 AM UTC, comment #2: 

sorry my sugestion  has problems.

Doug Stewart <dastew>
Sat 27 Dec 2014 02:07:51 AM UTC, comment #1: 

Goto line 156 in freqz.m and change it to:
 f = Fs * (0:n).' / N;


Is this what you want.
I am not the one to decide, just helping to clarify what you want.
What does Matlab do?

Doug Stewart <dastew>
Sat 27 Dec 2014 01:40:21 AM UTC, original submission:  

when I use freqz(num,den,10)

I expected the endpoints to be included.  I would think one would be interested in finite value (pi).  However, it only yields data thru 0.9*pi.  I would suggest adding/ensuring it also provides frequency response at pi (1 more step). 
So, freqz(num,den,2)  would produce (3) data values, not 2.
I often iterate things so as to match A/D response at the Nyquist frequency ...

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #33040:  freqz.patch added by dastew (2KiB - text/x-patch)
file #32990:  freqz.pdf added by lachlan (4KiB - application/pdf)
file #32951:  19638.patch added by dastew (2KiB - text/x-patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by lachlan (Updated the item)
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by dastew (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only group members can vote.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2015-02-10 rik5 StatusConfirmed Fixed
        Open/ClosedOpen Closed
    2015-02-09 dastew Attached File- Added freqz.patch, #33040
    2015-02-04 lachlan Attached File- Added freqz.pdf, #32990
    2015-01-30 dastew Attached File- Added 19638.patch, #32951
    2014-12-29 rik5 StatusNone Confirmed
        Summaryfreqz may work as designed but freqz plot should extend to end of range, not 1 data point short.

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code