On Fri, Jun 23, 2017 at 6:05 AM, Nicholas Jankowski <
address@hidden> wrote:
>
> On Jun 23, 2017 2:40 AM, "je suis" <
address@hidden> wrote:
>
> If you
> find that the script works (and checks against Matlab), then I'll
> submit to bugs.octave.
>
> BTW: should it be "Octave function" or "Octave Forge package"? And last release?
>
> Vlad
>
>
> I'll try to do some MATLAB checks later today. For the bug submission I guess this would be octave forge package since its a signal package function and release 4.2.1 (the version of octave you saw the bug in)
So, first, I see you still don't have any tests at the end that actually test for an expected output. Hence, even though you pass 23 out of 23 tests, the first attempt I make to run your first example gives:
>> firls (30, [0, 0.3, 0.4, 1], [0, 0, 1, 1])
error: Invalid call to abs. Correct usage is:
-- abs (Z)
error: called from
print_usage at line 91 column 5
firls at line 231 column 3
on line 231 you give:
K = [-abs (K); abs (K)](:)';
Any expected output test would have caught this. This error stems from is one of the exceptions given in the style guide:
"An exception are matrix or cell constructors:
[sin(x), cos(x)]
{sin(x), cos(x)}
Here, putting spaces after sin, cos would result in a parse error."
i.e., it sees [sin(x), cos(x)] as two array elements but [sin (x), cos (x)] as four, and sin/cos called with no arguments.
I saw the same error with a flipud (a).
Deleting the extra spaces lets the function run. Then, that first test appears to match matlab's output. I've also added other tests to the end that I had a while back in the other email thread. they check 'row vector output', a very simple 2-element function output, a NaN input that doesn't error on Matlab. They all pass. finally, I made a couple detailed output tests from matlab output of the examples you give in help. some pass. not all. I left one in that fails.
Attached is the modified firls.m, and a text file with 'expected' outputs for all of the examples you include in the help. I don't have time at the moment to make tests out of them all, but you may want to see which ones pass or fail, and see if there's an obvious reason in the algorithm.
did you create a bug report? now is as good a time as any. when you do, add me to the Mail Notification CC part at the bottom (user: nrjank)
So far, it's coming along very nicely. Good work.
Nick J.
firls.m