[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/