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

 
 

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

Submitted by:  None
Submitted on:  Tue 20 Feb 2018 05:39:17 PM UTC  
 
Category:  Forge : new function Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  Mike Miller <mtmiller> Originator Email:  -email is unavailable-
Open/Closed:  Closed

Add a New Comment (Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

( 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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.
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)
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    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.5