octave-patch-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Octave-patch-tracker] [patch #8078] [signal package new function] ultrw


From: Rob Sykes
Subject: [Octave-patch-tracker] [patch #8078] [signal package new function] ultrwin.m
Date: Wed, 15 Jan 2014 20:03:03 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Ubuntu Chromium/31.0.1650.63 Chrome/31.0.1650.63 Safari/537.36

Follow-up Comment #2, patch #8078 (project octave):

> * I prefer Octave coding standards for the signal package (# comments,
endif,
> endfunction, and GNU style for spacing and indentation). I can clean these
up
> easily enough when I incorporate your patch.

Hi Mike, certainly, this is appropriate for the .m & .cc files. 

> * Is there any reason not to combine ultrwin.c and ultrwin_.cc into one
file?

Perhaps; ultrwin.[ch] can be used by any language that can wrap or link-to C,
so I was hoping to point other projects at these files. (Being 'just a
window', it didn't seem to warrant a dedicated repo somewhere). But if it's a
problem, no worries, go ahead and combine; a 'cut here' comment can be fine.

> * We prefer to use leading and trailing double underscores for private
> functions, so the compiled function should be __ultrwin__ instead of
ultrwin_

Yes, of course.

> * It would probably make it clearer to split this into two patches, one
> introducing the new function and one reimplementing chebwin based on
ultrwin.

No problem with that.

> Please reply to discuss or if you want to post updated patches to this
tracker
> item. In the mean time I'll test actually using the new function and make 
> sure
> it works for me.

To compare the old/new chebwin, you could try something like:

tic; b=chebwin(1e7,300); toc; [h,f]=freqz(b,1,2^14); plot(f/pi,
20*log10(abs(h/h(1))))

Here, the new chebwin is faster and more accurate.

BTW, I've spotted a typo in the ultrwin.m documentation: "<= 1.5" should be
"<= -1.5".

Also, if the name 'ultrwin' is too terse, please choose something that
satisfies current Octave naming guidelines.

    _______________________________________________________

Reply to this item at:

  <http://savannah.gnu.org/patch/?8078>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.gnu.org/




reply via email to

[Prev in Thread] Current Thread [Next in Thread]