patchGNU Octave - Patches: patch #8506, [octave forge] (signal) taylorwin:...

 
 

patch #8506: [octave forge] (signal) taylorwin: new function

Submitter:  None
Submitted:  Wed 06 Aug 2014 12:03:00 AM UTC
   
 
Category:  Forge : new function Priority:  5 - Normal
Status:  Postponed Privacy:  Public
Assigned to:  mtmiller Originator Email:  -email is unavailable-
Open/Closed:  Open
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Wed 22 Apr 2020 03:29:06 PM UTC, comment #7: 

What is the current status for taylorwin? Anyone still trying to get this into Octave?

 Jim Greenlee <greenlej>
Sat 31 Mar 2018 07:12:27 AM UTC, comment #6: 

No problem.

I can do it. I think it is good experience to do it. I would like to rent the referance book again before starting though, but in the mean time I will read some documentation test blocks.

 Robert <i1nk>
Fri 30 Mar 2018 01:50:03 AM UTC, comment #5: 

Sorry for the very long delay. I have reviewed the latest patch here (file #31986).

The function still needs some formatting work, in the code, the doc strings, and the comments.

The functions a_constant and b_constant don't really need to be separated out. The other helper functions may be useful but they are written in an unnecessarily obtuse style.

I wouldn't be surprised if the for loop at the end of the function could be simplified and vectorized.

It would be helpful if the series of test cases written at the end of the function were actual %!test blocks.

I will come back to this and work on it some more, but help with any or all of these improvements would be welcome.

 Mike Miller <mtmiller>
Group Member
Wed 27 Aug 2014 09:56:47 PM UTC, comment #4: 

Added more test cases and change the error message for the SLL.

The test cases that were added is the ones that will make sure that the function can catch a non scalar N value.

The SLL message was saying SLL must be a negative value, but 0 is fine, so I changed the wording on the error message along with the help message to state so.

Also added error checking to ensure that all the input arguments are scalar values. (Forgot to check this, I was only looking at the size (m) and not n_bar and sll).

(file #31986)

 Robert <i1nk>
Wed 27 Aug 2014 02:53:37 AM UTC, comment #3: 

I used the file tests.m to compare the answer from taylorwinC.m to Matlab's taylorwin function. taylorwinC is the same as taylorwin but with matlab syntax and less error checking, but the error checking will not affect the answer in anyway. Matlabs version that I tested it with is also in the test.m along with the answers. Other than that I am not sure on how to verify that the function produces the intended values. The only other method I can thing of is to do the math by hand.

I did add the test cases to test that the invalid input would be caught. It is at the end along with a description on what it is catching.

(file #31971, file #31972, file #31973)

 Robert <i1nk>
Mon 18 Aug 2014 01:09:16 AM UTC, comment #2: 

Ok, I'll start on making those changes.

 Robert <i1nk>
Sun 17 Aug 2014 07:47:43 PM UTC, comment #1: 

Thanks for your contribution to Octave! This function can definitely be added to the signal package. Would you mind looking at some coding style fixups and adding unit tests?

It would be best for all new functions added to the signal package to adhere to GNU Octave coding style conventions. Here are some suggestions based on a quick review of your submission:

  • Use 2 spaces for indentation
  • Remove trailing spaces from all lines
  • Use endfunction instead of end
  • Omit brackets for single return values
  • Use space before opening parenthesis on function calls, if, switch, etc, no space when indexing
  • For loops: use parentheses on initializers, omit ":1:" for default case of a step-by-one iteration
  • Keep parentheses on function calls with no arguments (e.g. print_usage ())
  • Use "## " for function and in-code comments
  • Make sure to indicate the function name in error messages


Some simple particulars about this function:

  • Just error if the length argument is not a positive integer, refer to other window functions in the signal package for example argument error checking, for example https://hg.mtmxr.com/octave-signal/file/817a7a769729/inst/chebwin.m
  • Use m for the length argument to be consistent with other window functions in the package
  • Prefer lower case variable and inner function names, unless you want to differentiate matrices from scalars using uppercase local variable names


And add test cases if you can, to verify that the function produces the intended values and that it catches errors when given invalid input.

 Mike Miller <mtmiller>
Group Member
Wed 06 Aug 2014 12:03:00 AM UTC, original submission:  

I would like to add the taylorwin function to the signal package. The function was based heavily off the book titled Spotlight Synthetic Aperture Radar Signal Processing Algorithms, By Walter G. Carrara, Ron S. Goodman, and Ronald M. Majewski, and the is ISBN 0-89006-728-7 from Pg 512 - 513.

I broke up each part of the taylor window into smaller functions which call each other when they to be calculated. Also the constants are saved off and passed to the functions if they need that constant to find some other value.

Currently, this function will not work in matlab without changing some of the code style. But it works fine in octave.

I have tested a few cases and compared a few answers to the matlab function.

 Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #31986:  taylorwin.m added by i1nk (7KiB - text/x-objcsrc - Added more error checking and fix the SLL error message.)
file #31971:  taylorwin.m added by i1nk (7KiB - text/x-objcsrc)
file #31972:  tests.m added by i1nk (3KiB - text/x-objcsrc)
file #31973:  taylorwinC.m added by i1nk (2KiB - text/x-objcsrc)
file #31846:  taylorwin.m added by None (7KiB - text/x-objcsrc - This is the function for the patch)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by greenlej (Posted a comment)
  • -email is unavailable- added by i1nk (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 logged-in users can vote.

     

    Follow 12 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-03-07 mtmiller Carbon-CopyRemoved 80942 -
    2018-03-30 mtmiller Summarysignal package: add the taylorwin function [octave forge] (signal) taylorwin: new function
    2018-03-30 mtmiller CategoryNone Forge : new function
        StatusIn Progress Postponed
    2015-12-22 mtmiller SummaryAdd the taylorwin function to the signal package. signal package: add the taylorwin function
    2014-08-27 i1nk Attached File- Added taylorwin.m, #31986
    2014-08-27 i1nk Attached File- Added tests.m, #31972
        Attached File- Added taylorwinC.m, #31973
        Attached File- Added taylorwin.m, #31971
    2014-08-17 mtmiller StatusNone In Progress
        Assigned toNone mtmiller
    2014-08-06 None Attached File- Added taylorwin.m, #31846

    Back to the top

    Copyright © 2024 Free Software Foundation, Inc.
    Verbatim copying and distribution of this entire article is permitted in any medium, provided this notice is preserved.
    The Levitating, Meditating, Flute-playing Gnu logo is a GNU GPL'ed image provided by the Nevrax Design Team.
    Page source code

    Powered by Savane 3.14-f13d.
    Corresponding source code