patchGNU Octave - Patches: patch #9570, [octave forge] (signal) cconv: new...

 
 

patch #9570: [octave forge] (signal) cconv: new function

Submitter:  None
Submitted:  Tue 20 Feb 2018 05:39:17 PM UTC
   
 
Category:  Forge : new function Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  mtmiller Originator Email:  -email is unavailable-
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 02 Apr 2018 08:11:13 PM UTC, comment #9: 

The GPL header is not quite the same, it's more than just line breaks. Anyway, thank you for the changes, I have added this function to the signal package

https://hg.mtmxr.com/octave-signal/rev/787912148e02

I also made some small but important style changes and removed trailing whitespace

https://hg.mtmxr.com/octave-signal/rev/bd9f895d2e77

And lastly I found a bug when the input is complex and fixed it here with some additional tests

https://hg.mtmxr.com/octave-signal/rev/5e3061733a6d

Thanks for your contribution!

Mike Miller <mtmiller>
Group Member
Mon 02 Apr 2018 03:08:48 PM UTC, comment #8: 

GPLv3 license header is the same... only different line breaks.
Anyway... just copied from a file in repository to match the line breaks as well.

(file #43769)

Leonardo <leolca>
Mon 02 Apr 2018 03:04:01 PM UTC, comment #7: 

ok! it follows attached

(file #43768)

Leonardo <leolca>
Sat 31 Mar 2018 06:53:12 AM UTC, comment #6: 

Would you mind running the doctest function on this and rewrite the examples so that they pass doctest?


>> pkg install -forge doctest
>> doctest cconv
... FAIL 3/3 ...


The GPLv3 license header is also out of date, I can easily rewrite it with the latest, but if you wouldn't mind updating that, that would help (see any .m file in the signal package repository).

Mike Miller <mtmiller>
Group Member
Mon 12 Mar 2018 06:20:22 PM UTC, comment #5: 

The previous post was indeed using matrix method.

The FOR loop was performing the matrix method... as suggested, now it is using iFFT and FFT.

The first IF just add zeros for different length vectors.
The second IF fold the vectors into themselves in case the N (modulo) is smaller than the vector lengths, or add zeros if necessary.

(file #43530)

Leonardo <leolca>
Mon 12 Mar 2018 06:09:43 PM UTC, comment #4: 

Leonardo, so the code you shared via email seems perfect to me, would you consider submitting a patch here for everyone to see?

P Sudeepam <sudeepam>
Mon 12 Mar 2018 05:53:57 PM UTC, comment #3: 

Can you both please keep the discussion on the patch tracker? All of your reply-by-email comments are lost.

Mike Miller <mtmiller>
Group Member
Mon 12 Mar 2018 04:28:44 PM UTC, comment #2: 

Which algorithm have you used to calculate the Circular Convolution in this implementation? From the code it seems like matrix method has been used but I'd like to make sure.

P Sudeepam <sudeepam>
Tue 20 Feb 2018 05:54:30 PM UTC, comment #1: 

Thanks for your submission. You have done a pretty good job of adhering to the Octave preferred coding style, thank you for that. There are still some compact if-else statements that should be expanded, some missing spaces around operators. If you wouldn't mind looking at cleaning it up a bit, that would help, otherwise the person who commits it can clean it up.

Mike Miller <mtmiller>
Group Member
Tue 20 Feb 2018 05:39:17 PM UTC, original submission:  

Here is a contribution, an implementation of Modulo-N circular convolution: cconv.m.

Anonymous

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #43769:  cconv.m added by leolca (3KiB - text/x-objcsrc)
file #43768:  cconv.m added by leolca (3KiB - text/x-objcsrc)
file #43530:  cconv.m added by leolca (3KiB - text/x-objcsrc)
file #43375:  cconv.m added by None (3KiB - text/x-objcsrc - ok. I've just added extra spaces.)
file #43373:  cconv.m added by None (3KiB - text/x-objcsrc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by leolca (Updated the item)
  • -email is unavailable- added by sudeepam (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-02-26 mtmiller Carbon-CopyRemoved 80942 -
    2018-04-02 mtmiller StatusIn Progress Done
        Open/ClosedOpen Closed
    2018-04-02 leolca Attached File- Added cconv.m, #43769
    2018-04-02 leolca Attached File- Added cconv.m, #43768
    2018-03-31 mtmiller StatusNone In Progress
    2018-03-31 mtmiller Summary[octave forge] (signal) new function cconv: modulo-N circular convolution [octave forge] (signal) cconv: new function
    2018-03-31 mtmiller Assigned toNone mtmiller
    2018-03-12 leolca Attached File- Added cconv.m, #43530
    2018-02-20 None Attached File- Added cconv.m, #43375
    2018-02-20 mtmiller SummaryModulo-N circular convolution: cconv.m [octave forge] (signal) new function cconv: modulo-N circular convolution
    2018-02-20 None Attached File- Added cconv.m, #43373

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code