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

Submitter:  Nicholas Jankowski <nrjank>
Submitted:  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
* Mandatory Fields

Add a New Comment (Rich Markup)
   

Comment Type & Canned Response:
       

 

( 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>
Project Member
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>
Project Member
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>
Project Member
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>
Project Member
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>
Project Member
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>
Project Member

 

(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
  •  

    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 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.9