bugGNU Octave - Bugs: bug #49990, [octave forge] (signal)...

 
 

bug #49990: [octave forge] (signal) butter/ellip/cheby1/cheby2 error if only asking for a single output

Submitted by:  Nicholas Jankowski <nrjank>
Submitted on:  Thu 05 Jan 2017 07:31:11 PM UTC  
 
Category:  Octave Forge Package Severity:  3 - Normal
Priority:  5 - Normal Item Group:  Matlab Compatibility
Status:  Fixed Assigned to:  None
Originator Name:  Nicholas Jankowski Open/Closed:  Closed
Release:  4.2.0 Operating System:  Any

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( Jump to the original submission)

Wed 04 Apr 2018 01:18:41 AM UTC, comment #9: 

This is fixed in the signal package repository

https://hg.mtmxr.com/octave-signal/rev/88e8c154dcdb

Thank you for working on this patch. I made some very minor whitespace cleanup and minor additional style changes before merging it.

Mike Miller <mtmiller>
Project Administrator
Fri 06 Jan 2017 03:38:25 AM UTC, comment #8: 

attached.

(file #39386)

Nicholas Jankowski <nrjank>
Thu 05 Jan 2017 10:46:31 PM UTC, comment #7: 

Thanks. I would clean this up a bit, feel free to update your patch if you want to help me out, otherwise I can take care of it.

  • use "(bug #nnn, bug #nnn)" in the summary line to make hg history more searchable
  • no need for the explanatory comments about orientation or "fix bug#nnn" in the code, those should be explained by looking at the hg history
  • simplify to just have a case for "if (nargout <= 2)", it won't cause an error to assign to both a and b even if nargout is only 1 (maybe efficiency, but that doesn't seem to be a concern here)
  • don't add a space when indexing with "(:)", we only add space before function call arguments, not indexing operations
  • in the commit message, if the change is the same for all four files, just say "* butter.m, cheby1.m, cheby2.m, ellip.m: ..."
  • blank line between the summary line and the bullet lines in the commit message

Thanks

Mike Miller <mtmiller>
Project Administrator
Thu 05 Jan 2017 09:52:07 PM UTC, comment #6: 

I think this should do it. attached.

(file #39383)

Nicholas Jankowski <nrjank>
Thu 05 Jan 2017 09:20:37 PM UTC, comment #5: 

Cool. Can you make all four of these diffs into a single hg changeset?

Mike Miller <mtmiller>
Project Administrator
Thu 05 Jan 2017 09:12:01 PM UTC, comment #4: 

also forgot nargout=1 is not handled the same as nargout=0. all tests pass now.  output orientation is correct and works for nargout<2.  v2 diffs attached.

(file #39378, file #39379, file #39380, file #39381)

Nicholas Jankowski <nrjank>
Thu 05 Jan 2017 08:53:34 PM UTC, comment #3: 

forgot about the top usage check. those fixes take care of bug #49318 but not this one yet.

Nicholas Jankowski <nrjank>
Thu 05 Jan 2017 08:40:38 PM UTC, comment #2: 

diff posted addresses this bug and bug #49318 (output orientation) including verification tests for cheby1, cheby2, ellip and butter.

(file #39374, file #39375, file #39376, file #39377)

Nicholas Jankowski <nrjank>
Thu 05 Jan 2017 07:43:55 PM UTC, comment #1: 

Please be sure to include all four IIR functions in any changes addressing this.

Mike Miller <mtmiller>
Project Administrator
Thu 05 Jan 2017 07:31:11 PM UTC, original submission:  

signal package cheby2 function produces an error for nargout ==1, Matlab provides an output.

Octave 4.2.0, signal 1.3.2

>> Q = cheby2(3,4,.5)
error: Invalid call to cheby2.  Correct usage is:
...

Matlab 2016b:

>> Q = cheby2(3,4,.5)
Q =
    0.5817    0.7479    0.7479    0.5817

>> [b,a]=cheby2(3,4,.5)
b =
    0.5817    0.7479    0.7479    0.5817
a =
    1.0000    0.5929    0.7382    0.3279

>> Q==b
ans =
  1×4 logical array
   1   1   1   1

from cheby2 code it looks like it should be a simple add at the end. I'm putting together a set of simple output tests/hacks for bug #49318.  will try to include this.

Nicholas Jankowski <nrjank>

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #39386:  signal-fixes_v2.diff added by nrjank (7KiB - application/octet-stream)
file #39383:  signal-fixes.diff added by nrjank (8KiB - application/octet-stream - hg changeset covering all 4 files.)
file #39378:  ellip_fix_v2.diff added by nrjank (1KiB - application/octet-stream - corrected patches. all tests pass.)
file #39379:  butter_fix_v2.diff added by nrjank (1KiB - application/octet-stream - corrected patches. all tests pass.)
file #39380:  cheby1_fix_v2.diff added by nrjank (1KiB - application/octet-stream - corrected patches. all tests pass.)
file #39381:  cheby2_fix_v2.diff added by nrjank (1KiB - application/octet-stream - corrected patches. all tests pass.)
file #39374:  cheby2_fix.diff added by nrjank (3KiB - application/octet-stream - diffs for all four functions correcting output orientation and nargout=1 cases, with tests)
file #39375:  ellip_fix.diff added by nrjank (2KiB - application/octet-stream - diffs for all four functions correcting output orientation and nargout=1 cases, with tests)
file #39376:  cheby1_fix.diff added by nrjank (2KiB - application/octet-stream - diffs for all four functions correcting output orientation and nargout=1 cases, with tests)
file #39377:  butter_fix.diff added by nrjank (1019B - application/octet-stream - diffs for all four functions correcting output orientation and nargout=1 cases, with tests)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jwe (Updated the item)
  • -email is unavailable- added by mtmiller (Posted a comment)
  • -email is unavailable- added by nrjank (Submitted the item)
  • -email is unavailable- added by nrjank
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only project members can vote.

     

     

     

    Follow 18 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2018-07-02 jwe Summary[octave-forge] (signal) butter/ellip/cheby1/cheby2 error if only asking for a single output => [octave forge] (signal) butter/ellip/cheby1/cheby2 error if only asking for a single output
    2018-04-04 mtmiller StatusPatch Submitted => Fixed
        Open/ClosedOpen => Closed
    2017-08-11 jwe Summarysignal package: butter/ellip/cheby1/cheby2 error if only asking for a single output => [octave-forge] (signal) butter/ellip/cheby1/cheby2 error if only asking for a single output
    2017-01-06 nrjank Attached File- => Added signal-fixes_v2.diff, #39386
    2017-01-05 mtmiller StatusConfirmed => Patch Submitted
    2017-01-05 nrjank Attached File- => Added signal-fixes.diff, #39383
    2017-01-05 nrjank Attached File- => Added ellip_fix_v2.diff, #39378
        Attached File- => Added butter_fix_v2.diff, #39379
        Attached File- => Added cheby1_fix_v2.diff, #39380
        Attached File- => Added cheby2_fix_v2.diff, #39381
    2017-01-05 nrjank Attached File- => Added cheby2_fix.diff, #39374
        Attached File- => Added ellip_fix.diff, #39375
        Attached File- => Added cheby1_fix.diff, #39376
        Attached File- => Added butter_fix.diff, #39377
    2017-01-05 mtmiller StatusNone => Confirmed
        Summarycheby2 errors out if only asking for a single output. => signal package: butter/ellip/cheby1/cheby2 error if only asking for a single output
    2017-01-05 nrjank Carbon-Copy- => Added mtmiller

    Back to the top


    Powered by Savane 3.5