patchGNU Octave - Patches: patch #8864, add smooth3

 
 

patch #8864: add smooth3

Submitter:  Markus Mützel <mmuetzel>
Submitted:  Mon 18 Jan 2016 12:05:03 PM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  None Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Mon 18 Jul 2016 10:44:25 PM UTC, comment #6: 

I committed your patch to add smooth3 to Octave in this cset (http://hg.savannah.gnu.org/hgweb/octave/rev/62208397b99e).

Some changes:

1) No validation of nargout.  We check for the correct number of input arguments, but don't bother to check outputs.  If a user calls a function which doesn't return enough outputs they will get a message from the interpreter about that error already.

2) Simplified input processing by using default arguments to function.  Instead of


  if (nargin < 2), method = "box"; endif
  if (nargin < 3), sz = 3; endif
  if (nargin < 4), st_dev = .65; endif


I just declared the function as used


function smoothed_data = smooth3 (data, method = "box", sz = 3, std_dev = 0.65)


3) I tightened up the input validation.  For example, there was no checking that METHOD was a string before using it in the switch statement which caused problems when it was a cell.  And originally the input matrix was only tested for size, but a cell array of the right dimensions would pass so I added isnumeric


  if (! isnumeric (data) || ndims (data) != 3)
    error ("smooth3: DATA must be a 3-D numeric matrix");


4) I used in-place operators where possible because they are much more efficient.


<  gaussian3 /= sum (gaussian3(:));  # normalize
---
>  gaussian3 = gaussian3 / sum (gaussian3(:)); ## normalize


5) In the %!demo blocks I used Matlab syntax so that they will run correctly under Matlab for the compare_plot_demos script.  This meant changing double quotes to single quotes.

6) I change the %!error tests to be smaller and fit on a single line.  This was mostly just about being concise and having line lengths less than 80 characters.

Rik <rik5>
Group administrator
Mon 18 Jul 2016 05:44:06 PM UTC, comment #5: 

The attached patch removes "Function File" from the deftypefns and converts the example in the previous patch to a demo (thus coincidentally removing overlong lines in the docstring). Other than that only minor changes to the docstring.
This patch should apply cleanly on the side of patch #9040 "lighting".

(file #37935)

Markus Mützel <mmuetzel>
Group administrator
Wed 06 Jul 2016 08:02:16 PM UTC, comment #4: 

Lachlan, attached please find the function "smooth3" as a complete changeset.

(file #37738)

Markus Mützel <mmuetzel>
Group administrator
Wed 06 Jul 2016 11:18:39 AM UTC, comment #3: 

Markus, if you would like this to be considered for 4.2.0, could you please prepare a complete cset.

Lachlan Andrew <lachlan>
Tue 19 Jan 2016 08:31:54 PM UTC, comment #2: 

If with "Is it possible to read from .mat file in the test" you mean to have a pre-cooked .mat file somewhere, well that is obviously possible. But...

In an Octave-Forge package (not in core Octave) it's easy to have a .mat file in e.g., a demo or test subdir of the package; the file can be used for a demo as well as for a test then.
I'm not sure if this, or a similar solution, is possible in core Octave.

In the io package (that I maintain) there are file templates in a "template" subdir.

BTW, as to tests, on Linux:

>> test smooth3
PASSES 19 out of 19 tests


Nice!

Philip Nienhuis <philipnienhuis>
Group Member
Tue 19 Jan 2016 04:38:32 PM UTC, comment #1: 

I changed the example to one that better shows what 'smooth3' does.

Additionally, I expanded the "see also" list to include more related functions and added a check of the input/output argument count.

The rest of the changes to the prior patch are only a little bit of code cleaning.

The issues(?) described in #comment0 still exist.

(file #36094)

Markus Mützel <mmuetzel>
Group administrator
Mon 18 Jan 2016 12:05:03 PM UTC, original submission:  

This is a patch to add the function smooth3 to Octave.

I did some tests to compare the results in Matlab and Octave. But the results differ slightly. The deviations are up to 5.5*eps in one example (see attachments smooth3_test.m and smooth3_data.mat). They might be higher in other cases.

I am not quite sure where the small deviations come from. Maybe Matlab and Octave use different ways to save to and load data from mat-files. Maybe there are differences in the implementation of some functions that are used (could be "exp" and "convn"). Or they have a completely different reason...

I can test this only on Windows and do not know whether the results are different on other platforms.

Are these small deviations an issue?

I do not know how to implement these manual tests as automated tests at the end of the .m-file. Is it possible to read from .mat-files in the test?

Meanwhile, I added some tests that assert the size of the resulting matrix but not really its content.

Markus Mützel <mmuetzel>
Group administrator

 

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

Attach Files:
   
   
Comment:
   

Attached Files
file #37935:  smooth3_v2.patch added by mmuetzel (10KiB - text/x-diff)
file #37738:  smooth3.patch added by mmuetzel (10KiB - text/x-diff)
file #36094:  smooth3.m added by mmuetzel (8KiB - text/plain)
file #36078:  smooth3.m added by mmuetzel (8KiB - text/x-objective-c)
file #36079:  smooth3_data.mat added by mmuetzel (45KiB - application/octet-stream)
file #36080:  smooth3_test.m added by mmuetzel (611B - text/x-objective-c)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by rik5 (Posted a comment)
  • -email is unavailable- added by mmuetzel
  • -email is unavailable- added by lachlan (Posted a comment)
  • -email is unavailable- added by mmuetzel (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 9 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-18 rik5 StatusNone Done
        Open/ClosedOpen Closed
    2016-07-18 mmuetzel Attached File- Added smooth3_v2.patch, #37935
        Carbon-Copy- Added philipnienhuis
    2016-07-06 mmuetzel Attached File- Added smooth3.patch, #37738
    2016-01-19 mmuetzel Attached File- Added smooth3.m, #36094
    2016-01-18 mmuetzel Attached File- Added smooth3.m, #36078
        Attached File- Added smooth3_data.mat, #36079
        Attached File- Added smooth3_test.m, #36080

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code