changeset 26248:8a0778f549e8

movfun.m, movslice.m: Add additional input validation and BIST tests for same. * movfun.m: Rename "basefun" to "basefcn" in documentation. Validate number of inputs is at least 3. Use strcmpi, rather than ismember, in validation for performance. Use anonymous variable @(d), not @(x) when writing anonymous function that needs to check the existing variable 'x' so that the value is not shadowed. Use isindex() to verify indices for performance. Fix search for first non-singleton dimension so it returns a value even for scalar or empty matrix. Validate that WLEN is a 1- or 2-element array of integers >= 0. Use parfor, instead of for, for loop over columns. Eventually this might do something meaningful in Octave. Add BIST tests for input validation that check the expected error message. * movslice.m: Validate that exactly two arguments are provided. Validate that N is a positive integer. Validate that WLEN is a 1- or 2-element array of integers >= 0. Add BIST tests for input validation that check the expected error message.
author Rik <rik@octave.org>
date Sun, 16 Dec 2018 15:21:48 -0800
parents 86217421a37f
children 78c4aadfbfd9
files scripts/signal/movfun.m scripts/signal/movslice.m
diffstat 2 files changed, 93 insertions(+), 29 deletions(-) [+]
line wrap: on
line diff
--- a/scripts/signal/movfun.m	Sun Dec 16 13:53:54 2018 -0800
+++ b/scripts/signal/movfun.m	Sun Dec 16 15:21:48 2018 -0800
@@ -120,13 +120,13 @@
 ## is not an issue, the easiest way to select output dimensions is to first
 ## calculate the complete result with @code{movfun} and then filter that result
 ## with indexing.  If code complexity is not an issue then a wrapper can be
-## created using anonymous functions.  For example, if @code{basefun}
+## created using anonymous functions.  For example, if @code{basefcn}
 ## is a function returning a @var{K}-dimensional row output, and only
 ## dimension @var{D} is desired, then the following wrapper could be used.
 ##
 ## @example
 ## @group
-## @var{fcn} = @@(x) basefun (x)(:,size(x,2) * (@var{D}-1) + (1:size(x,2)));
+## @var{fcn} = @@(x) basefcn (x)(:,size(x,2) * (@var{D}-1) + (1:size(x,2)));
 ## @var{y} = movfun (@@fcn, @dots{});
 ## @end group
 ## @end example
@@ -136,6 +136,10 @@
 
 function y = movfun (fcn, x, wlen, varargin)
 
+  if (nargin < 3)
+    print_usage ();
+  endif
+
   valid_bc = {"shrink", "periodic", "same", "zero", "fill"};
 
   persistent dispatch;
@@ -151,13 +155,13 @@
   parser = inputParser ();
   parser.FunctionName = "movfun";
   parser.addParamValue ("Endpoints", "shrink", ...
-    @(x)any (ismember (tolower (x), valid_bc)));
+    @(x) any (strcmpi (x, valid_bc)));
   parser.addParamValue ("dim", [], ...
-    @(x) isempty(x) || (isscalar (x) && x > 0 && x <= ndims(x)));
+    @(d) isempty (d) || (isscalar (d) && isindex (d, ndims (x))));
   parser.addParamValue ("nancond", "omitnan", ...
-    @(x) any (ismember (x, {"omitnan", "includenan"})));
+    @(x) any (strcmpi (x, {"omitnan", "includenan"})));
   parser.addParamValue ("outdim", [], ...
-    @(x) isempty(x) || (isvector (x) && all (x > 0)));
+    @(d) isempty (d) || (isvector (d) && isindex (d)));
 
   parser.parse (varargin{:});
   bc      = parser.Results.Endpoints;   # boundary condition
@@ -167,32 +171,40 @@
   clear parser
   ## End parse input arguments
 
-  ## If dim was not provided search for the first non-singleton dimension
-  szx  = size (x);
+  ## If dim was not provided find the first non-singleton dimension.
+  szx = size (x);
   if (isempty (dim))
-    dim  = find (szx > 1, 1, "first");
+    (dim = find (szx > 1, 1)) || (dim = 1);
   endif
 
   ## Window length validation
+  if (! (isnumeric (wlen) && all (wlen >= 0) && fix (wlen) == wlen))
+    error ("Octave:invalid-input-arg",
+           "movfun: WLEN must be a scalar or 2-element array of integers >= 0");
+  endif
   if (isscalar (wlen))
     ## Check for proper window length
     ## FIXME: Matlab accepts even windows
     if (mod (wlen, 2) == 0)
       error ("Octave:invalid-input-arg", "movfun: WLEN must be an odd length");
+    elseif (wlen == 1)
+      error ("Octave:invalid-input-arg", "movfun: WLEN must be > 1");
     endif
-    if (wlen == 1)
-      error ("Octave:invalid-input-arg", "movfun: WLEN must be larger than 1");
-    endif
+  elseif (numel (wlen) == 2)
+    ## FIXME: Any further tests needed to validate form: wlen = [nb, na] ???
+  else
+    error ("Octave:invalid-input-arg",
+           "movfun: WLEN must be a scalar or 2-element array of integers >= 0");
   endif
-  ## FIXME: Need to validate form: wlen = [nb, na]
 
-  ## Check that array is longer that wlen at dim.  At least one full window
-  ## must fit. Function max is used to include the case when wlen is an array.
+  ## Check that array is longer than WLEN at dimension DIM.  At least one full
+  ## window must fit.  Function max is used to include the case when WLEN is an
+  ## array. 
   ## FIXME: Consider using bc to decide what to do here.
   if (max (wlen) > szx(dim))
       error ("Octave:invalid-input-arg", ...
-             "movfun: window length (%d) must be shorter than length along DIM (%d=%d)", ...
-        wlen, dim, szx(dim));
+             "movfun: window length WLEN (%d) must be shorter than length along DIM%d (%d)", ...
+             max (wlen), dim, szx(dim));
   endif
 
   ## Move the desired dim to the 1st dimension
@@ -209,6 +221,7 @@
   ## Obtain slicer
   [slc, C, Cpre, Cpos, win] = movslice (N, wlen);
 
+  ## FIXME: validation doesn't seem to work correctly
   ## Validate that outdim makes sense
   tmp     = fcn (zeros (length (win), 1));  # output for window
   noutdim = length (tmp);                   # number of output dimensions
@@ -231,12 +244,11 @@
 
   ## Apply processing to each column
   ## FIXME: Is it faster with cellfun?  Don't think so, but needs testing.
-  ## It could be parallel
   y = zeros (N, ncols, soutdim);
-  for i = 1:ncols
+  parfor i = 1:ncols
     y(:,i,:) = movfun_oncol (fcn_, x(:,i), wlen, bcfunc,
                              slc, C, Cpre, Cpos, win, soutdim);
-  endfor
+  endparfor
 
   ## Restore shape
   y = reshape (y, [szx(dperm), soutdim]);
@@ -246,6 +258,7 @@
 endfunction
 
 function y = movfun_oncol (fcn, x, wlen, bcfunc, I, C, Cpre, Cpos, win, odim)
+
   N = length (x);
   y = NA (N, odim);
 
@@ -554,8 +567,21 @@
 %!error (movfun (@(x) [min(x), max(x)], (1:10).', 3, "Outdim", 3))
 
 ## Test input validation
-%!error (movfun (@min, [0;0], 1))  # wlen == 1
-%!error (movfun (@min, [0;0], 2))  # odd wlen
-%!error (movfun (@min, [0;0], 3))  # wlen larger than data
-%!error (movfun (@min, [0;0;0], [1 4]))  # wlen larger than data
-%!error (movfun (@min, [0;0;0], [4 1]))  # wlen larger than data
+%!error movfun ()
+%!error movfun (@min)
+%!error movfun (@min, 1)
+%!error <WLEN must be .* array of integers> movfun (@min, 1, {1})
+%!error <WLEN must be .* array of integers .= 0> movfun (@min, 1, -1)
+%!error <WLEN must be .* array of integers> movfun (@min, 1, 1.5)
+%!error <WLEN must be an odd length> movfun (@min, 1, 4)
+%!error <WLEN must be . 1> movfun (@min, 1, 1)
+%!error <WLEN must be a scalar or 2-element array> movfun (@min, 1, [1, 2, 3])
+%!error <WLEN \(3\) must be shorter than length along DIM1 \(1\)>
+%! movfun (@min, 1, 3);
+%!error <WLEN \(4\) must be shorter than length along DIM1 \(1\)>
+%! movfun (@min, 1, [4, 1]);
+%!error <WLEN \(5\) must be shorter than length along DIM1 \(1\)>
+%! movfun (@min, 1, [1, 5]);
+## FIXME: This test is commented out until OUTDIM validation is clarified.
+%!#error <OUTDIM \(5\) is larger than largest available dimension \(3\)>
+%! movfun (@min, ones (6,3,4), 3, "outdim", 5);
--- a/scripts/signal/movslice.m	Sun Dec 16 13:53:54 2018 -0800
+++ b/scripts/signal/movslice.m	Sun Dec 16 15:21:48 2018 -0800
@@ -33,12 +33,39 @@
 
 function [slcidx, C, Cpre, Cpost, win] = movslice (N, wlen)
 
-  ## FIXME: Input validation for N, wlen
+  if (nargin != 2)
+    print_usage ();
+  endif
+
+  ## Validate N
+  if (! (isscalar (N) && isindex (N)))
+    error ("movslice: N must be a positive integer");
+  endif
 
-  ## FIXME: Eventually add asymmetric window
+  ## Validate window length
+  if (! (isnumeric (wlen) && all (wlen >= 0) && fix (wlen) == wlen))
+    error ("Octave:invalid-input-arg",
+           "movslice: WLEN must be a scalar or 2-element array of integers >= 0");
+  endif
+  if (isscalar (wlen))
+    ## Check for proper window length
+    ## FIXME: Matlab accepts even windows
+    if (mod (wlen, 2) == 0)
+      error ("Octave:invalid-input-arg", "movslice: WLEN must be an odd length");
+    elseif (wlen == 1)
+      error ("Octave:invalid-input-arg", "movslice: WLEN must be > 1");
+    endif
+  elseif (numel (wlen) == 2)
+    ## FIXME: Any further tests needed to validate form: wlen = [nb, na] ???
+  else
+    error ("Octave:invalid-input-arg",
+           "movfun: WLEN must be a scalar or 2-element array of integers >= 0");
+  endif
+
+  ## FIXME: Eventually add support for asymmetric window
   if (isscalar (wlen))
     hwlen = (wlen - 1) / 2;
-    wlen = [hwlen hwlen];
+    wlen = [hwlen, hwlen];
   endif
 
   Cpre  = 1:wlen(1);              # centers that can't fit the pre-window
@@ -51,4 +78,15 @@
 endfunction
 
 
-## FIXME: Need BIST tests
+## FIXME: Need BIST functional tests
+
+## Test input validation
+%!error movslice ()
+%!error movslice (1)
+%!error movslice (1,2,3)
+%!error <WLEN must be .* array of integers> movslice (1, {1})
+%!error <WLEN must be .* array of integers .= 0> movslice (1, -1)
+%!error <WLEN must be .* array of integers> movslice (1, 1.5)
+%!error <WLEN must be an odd length> movslice (1, 4)
+%!error <WLEN must be . 1> movslice (1, 1)
+%!error <WLEN must be a scalar or 2-element array> movslice (1, [1, 2, 3])