Re: goals for 3.1

[David Bateman]

John W. Eaton wrote:
> On  9-Mar-2008, David Bateman wrote:
>
> | John W. Eaton wrote:
> | >    7. Ensure that all operations which work on dimensions alone
> | >       (squeeze, numel, triu, etc.) work for all objects and preserve
> | >       type.  Should these functions all be built-in?  Possibly they
> | >       should all be provided by the octave_value class interface.
> | >   
> | 
> | Ok, I see this task as being in three parts, and maybe a zeroth part
> | 
> | 0) Identify which functions are concerned. I see the list at the moment
> | as being triu, tril, squeeze, permute, ipermute, shiftdim, circshift,
> | reshape,  and resize. I'm not sure I see why numel was in that original
> | list. Are there any others?
>
> That was a mistake.  It should have just been a list of functions that
> operate on dimensions or by indexing elements (like triu).
>
> What about diag, flipdim, fliplr, flipud?
>   
Ok, i'll add these and transpose, ctranspose, rot90, and a few others

> I'm not sure that all of these should be built in.  The ones that are
> .m files may already preserve type correctly, though it is worth
> checking, especially for any cases that can return empty values (they
> may convert to double then).  At least for now I would not bother to
> convert .m files to built-in functions unless there is a definite need
> for improved performance.
>   
Ok, I won't.. My first step is to propose the test code to prove that
the functions are currently implemented correctly or fix them if they
aren't.. I already sent one fix for circshift as the empty matrix case
caused issues, and the current test code doesn't identify any other issues.
> I have no objection to putting the tests in a separate file in the
> test directory if there is something to be gained by grouping the
> tests together.
>   
Ok, this is what I did in the attached patch, as it is much simpler.

> | 3) Convert these functions to be members of the octave_value class. What
> | are your thoughts now on this? Should they be members of the
> | octave_value class? Or can we forgo this?
>
> Let's look at each case individually.  If writing a function in the
> body of a DEFUN requires switching on type (for example) then I think
> we should use octave_value methods for that, possibly with the actual
> implementation in the Array/Sparse classes.
>   
The only function I see that is not already in the octave_value class
but is already a DEFUN is the Fdiag function. Yes this function has a
mess of if/else blocks for the types and so its probably worth moving it
into the octave_value class. I'll look at a patch for that in the near
future..

I attach a patch for Octave to add the test code and an equivalent 
function that can be used in Octave/Matlab to test compatible behavior.
Are there any other functions I missed? Are there any additional tests
that might be useful?

Regards
David


-- 
David Bateman                                address@hidden
Motorola Labs - Paris                        +33 1 69 35 48 04 (Ph) 
Parc Les Algorithmes, Commune de St Aubin    +33 6 72 01 06 33 (Mob) 
91193 Gif-Sur-Yvette FRANCE                  +33 1 69 35 77 01 (Fax) 

The information contained in this communication has been classified as: 

[x] General Business Information 
[ ] Motorola Internal Use Only 
[ ] Motorola Confidential Proprietary

# HG changeset patch
# User David Bateman <address@hidden>
# Date 1206023400 -3600
# Node ID 5d77961bc965edf97c4fadf618add369175bac16
# Parent  67577b66c89665b7e22f5abf17ea387d3a610619
Add tests for preservation of type for functions that work on dimensions of the 
matrices

diff --git a/test/ChangeLog b/test/ChangeLog
--- a/test/ChangeLog
+++ b/test/ChangeLog
@@ -1,3 +1,9 @@ 2008-03-07  John W. Eaton  <address@hidden
+2008-03-20  David Bateman  <address@hidden>
+
+       * test_func.m: New test code that ensures that all operations
+       which work on dimensions alone (squeeze, triu, etc.) work for all
+       objects and preserve type.
+
 2008-03-07  John W. Eaton  <address@hidden>
 
        * test_logical-wfi-t.m, test_logical-wfi-f.m: Update tests for
diff --git a/test/test_func.m b/test/test_func.m
new file mode 100644
--- /dev/null
+++ b/test/test_func.m
@@ -0,0 +1,187 @@
+## Copyright (C) 2008  David Bateman
+##
+## This file is part of Octave.
+##
+## Octave is free software; you can redistribute it and/or modify it
+## under the terms of the GNU General Public License as published by
+## the Free Software Foundation; either version 3 of the License, or (at
+## your option) any later version.
+##
+## Octave is distributed in the hope that it will be useful, but
+## WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+## General Public License for more details.
+##
+## You should have received a copy of the GNU General Public License
+## along with Octave; see the file COPYING.  If not, see
+## <http://www.gnu.org/licenses/>.
+
+## This piece of test code ensures that all operations which work on 
+## dimensions alone (squeeze, triu, etc.) work for all objects and 
+## preserve type. Even if the object is an empty matrix. This code is
+## not to check that the function itself returns teh correct result,
+## just that the results are consistent for all types.
+
+%!function __fntestfunc__ (fn, min, varargin)
+%!  typ = {'double', 'complex', 'logical', 'sparse', 'complex sparse', ...
+%!         'logical sparse', 'int8', 'int16', 'int32', 'int64', 'uint8', ...
+%!         'uint16', 'uint32', 'uint64'};
+%!
+%!  cmplx = [2, 5];
+%!  nlogical = [3, 6];
+%!  ninteger = [7, 8, 9, 10, 11, 12, 13, 14];
+%!  nsparse = [4, 5, 6];
+%!  usesparse = true;
+%!
+%!  if (length (varargin) > 0 && islogical (varargin{1}))
+%!    usesparse = varargin{1};
+%!    varargin(1) = [];
+%!  endif
+%!
+%!  for i = 1 : length(typ)
+%!    m = min;
+%!    if (length (varargin) > 0)
+%!      args = varargin(1);
+%!    else
+%!      args = {};
+%!    endif
+%!
+%!    if (any (nsparse == i))
+%!      if (! usesparse)
+%!        continue;
+%!      endif
+%!      if (ndims (m) > 2)
+%!        sz = size (m);
+%!        m = reshape (m, [sz(1), prod(sz (2:end))]);
+%!      endif
+%!      if (any (cmplx == i))
+%!        m = sparse ((1 + 1i) * m);
+%!      else
+%!        m = sparse (m);
+%!      endif
+%!    else
+%!      if (any (cmplx == i))
+%!        m = (1 + 1i) * m;
+%!      endif
+%!    endif
+%!    if (any (nlogical == i))
+%!      m = cast (m, 'logical');
+%!    endif
+%!    if (any (ninteger == i))
+%!      m = cast (m, typ{i});
+%!    endif
+%!
+%!    y = feval (fn, m, args{:});
+%!    if (!strcmp (class (y), class (m)) ||
+%!        issparse (y) != issparse (m) ||
+%!         any (cast (real (y), 'double')(:) !=
+%!              feval (fn , cast (real (m), 'double'), args{:})(:)))
+%!      error ('failed for type %s\n', typ{i});
+%!    endif
+%!  endfor
+%! endfunction
+
+%!shared m0, m1, m2, m3
+%! m0 = [1:5];
+%! m1 = reshape ([1 : 30], [5, 6]);
+%! m2 = reshape ([1 : 30], [5, 1, 6]);
+%! m3 = [];
+
+%!test
+%! __fntestfunc__('triu', m1);
+%!test
+%! __fntestfunc__ ('triu', m1, -1);
+%!test
+%! __fntestfunc__ ('triu', m1, 1);
+%!test
+%! __fntestfunc__('triu', m3);
+%!test
+%! __fntestfunc__ ('tril', m1);
+%!test
+%! __fntestfunc__ ('tril', m1, -1);
+%!test
+%! __fntestfunc__ ('tril', m1, 1);
+%!test
+%! __fntestfunc__('tril', m3);
+%!test
+%! __fntestfunc__ ('squeeze', m2);
+%!test
+%! __fntestfunc__ ('squeeze', m3);
+%!test
+%! __fntestfunc__ ('permute', m1, [2, 1]);
+%!test
+%! __fntestfunc__ ('permute', m2, false, [3, 1, 2]);
+%!test
+%! __fntestfunc__ ('permute', m3, [2, 1]);
+%!test
+%! __fntestfunc__ ('ipermute', m1, [2, 1]);
+%!test
+%! __fntestfunc__ ('ipermute', m2, false, [3, 1, 2]);
+%!test
+%! __fntestfunc__ ('ipermute', m3, [2, 1]);
+%!test
+%! __fntestfunc__ ('shiftdim', m2, 1);
+%!test
+%! __fntestfunc__ ('shiftdim', m2, false, -1);
+%!test
+%! __fntestfunc__ ('shiftdim', m3, 1);
+%!test
+%! __fntestfunc__ ('circshift', m2, 1);
+%!test
+%! __fntestfunc__ ('circshift', m2, [1, -1]);
+%!test
+%! __fntestfunc__ ('circshift', m3, 1);
+%!test
+%! __fntestfunc__ ('reshape', m2, [6, 5]);
+%!test
+%! __fntestfunc__ ('reshape', m3, [1, 0]);
+%!test
+%! __fntestfunc__ ('diag', m0);
+%!test
+%! __fntestfunc__ ('diag', m0, 1);
+%!test
+%! __fntestfunc__ ('diag', m0, -1);
+%!test
+%! __fntestfunc__ ('diag', m1);
+%!test
+%! __fntestfunc__ ('diag', m1, 1);
+%!test
+%! __fntestfunc__ ('diag', m1, -1);
+%!test
+%! __fntestfunc__ ('diag', m3);
+%!test
+%! __fntestfunc__ ('fliplr', m1);
+%!test
+%! __fntestfunc__ ('fliplr', m3);
+%!test
+%! __fntestfunc__ ('flipud', m1);
+%!test
+%! __fntestfunc__ ('flipud', m3);
+%!test
+%! __fntestfunc__ ('flipdim', m1, 2);
+%!test
+%! __fntestfunc__ ('flipdim', m3, 2);
+%!test
+%! __fntestfunc__ ('transpose', m1);
+%!test
+%! __fntestfunc__ ('transpose', m3);
+%!test
+%! __fntestfunc__ ('ctranspose', m1);
+%!test
+%! __fntestfunc__ ('ctranspose', m3);
+%!test
+%! __fntestfunc__ ('rot90', m1);
+%!test
+%! __fntestfunc__ ('rot90', m1, 2);
+%!test
+%! __fntestfunc__ ('rot90', m1, -1);
+%!test
+%! __fntestfunc__ ('rot90', m3);
+%!test
+%! __fntestfunc__ ('rotdim', m2, 1, [1, 2]);
+%!test
+%! __fntestfunc__ ('rotdim', m2, 2, [1, 2]);
+%!test
+%! __fntestfunc__ ('rotdim', m2, -1, [1, 2]);
+%!test
+%! __fntestfunc__ ('rotdim', m3, 1, [1, 2]);

function testfunc
  m0 = [1:5];
  m1 = reshape ([1 : 30], [5, 6]);
  m2 = reshape ([1 : 30], [5, 1, 6]);
  m3 = [];

  fntestfunc ('triu', m1);
  fntestfunc ('triu', m1, -1);
  fntestfunc ('triu', m1, 1);
  fntestfunc ('triu', m3);
  fntestfunc ('tril', m1);
  fntestfunc ('tril', m1, -1);
  fntestfunc ('tril', m1, 1);
  fntestfunc ('tril', m3);
  fntestfunc ('squeeze', m2);
  fntestfunc ('squeeze', []);
  fntestfunc ('permute', m1, [2, 1]);
  fntestfunc ('permute', m2, false, [3, 1, 2]);
  fntestfunc ('permute', m3, [2, 1]);
  fntestfunc ('ipermute', m1, [2, 1]);
  fntestfunc ('ipermute', m2, false, [3, 1, 2]);
  fntestfunc ('ipermute', m3, [2, 1]);
  fntestfunc ('shiftdim', m2, 1);
  fntestfunc ('shiftdim', m2, false, -1);
  fntestfunc ('shiftdim', m3, 1);
  fntestfunc ('circshift', m2, 1);
  fntestfunc ('circshift', m2, [1, -1]);
  fntestfunc ('circshift', m3, 1);
  fntestfunc ('reshape', m2, [6, 5]);
  fntestfunc ('reshape', m3, [1, 0]);
  fntestfunc ('diag', m0);
  fntestfunc ('diag', m0, 1);
  fntestfunc ('diag', m0, -1);
  fntestfunc ('diag', m1);
  fntestfunc ('diag', m1, 1);
  fntestfunc ('diag', m1, -1);
  fntestfunc ('diag', m3);
  fntestfunc ('fliplr', m1);
  fntestfunc ('flipud', m1);
  fntestfunc ('flipdim', m1, 2);
  fntestfunc ('transpose', m1);
  fntestfunc ('ctranspose', m1);
  fntestfunc ('rot90', m1);
  fntestfunc ('rot90', m1, 2);
  fntestfunc ('rot90', m1, -1);

  %% Only exists in Octave
  if (exist ('OCTAVE_VERSION'))
    fntestfunc ('rotdim', m2, 1, [1, 2]);
    fntestfunc ('rotdim', m2, 2, [1, 2]);
    fntestfunc ('rotdim', m2, -1, [1, 2]);
  end
end

function fntestfunc (fn, mn, varargin)
  typ = {'double', 'complex', 'logical', 'sparse', 'complex sparse', ...
         'logical sparse', 'int8', 'int16', 'int32', 'int64', 'uint8', ...
         'uint16', 'uint32', 'uint64'};

  cmplx = [2, 5];
  nlogical = [3, 6];
  ninteger = [7, 8, 9, 10, 11, 12, 13, 14];
  nsparse = [4, 5, 6];
  vec = @(x) x(:);
  usesparse = true;

  if (length (varargin) > 0 && islogical (varargin{1}))
    usesparse = varargin{1};
    varargin(1) = [];
  end

  fprintf ('Testing %s:\n', fn);
 
  for i = 1 : length(typ)
    m = mn;
    if (length (varargin) > 0)
      args = varargin(1);
    else
      args = {};
    end

    if (any (nsparse == i))
      if (~ usesparse)
        continue;
      end
      if (ndims (m) > 2)
        sz = size (m);
        m = reshape (m, [sz(1), prod(sz (2:end))]);
      end
      if (any (cmplx == i))
        m = sparse ((1 + 1i) * m);
      else
        m = sparse (m);
      end
    else
      if (any (cmplx == i))
        m = (1 + 1i) * m;
      end
    end
    if (any (nlogical == i))
      m = cast (m, 'logical');
    end
    if (any (ninteger == i))
      m = cast (m, typ{i});
    end

    try
      y = feval (fn, m, args{:});
      if (~strcmp (class (y), class (m)) || ...
               issparse (y) ~= issparse (m) || ...
             any (vec (cast (real (y), 'double')) ~= ...
               vec (feval (fn , cast (real (m), 'double'), args{:}))))
        error ();
      end
    catch
           ~strcmp (class (y), class (m))
           issparse (y) ~= issparse (m)
           any (vec (cast (real (y), 'double')) ~= ...
                vec (feval (fn , cast (real (m), 'double'), args{:})))


      fprintf ('   failed for type %s\n', typ{i});
    end
    if (exist ('OCTAVE_VERSION'))
      eval ('fflush(stdout);');
    end
  end
end

%!function __fntestfunc__ (fn, min, varargin)
%!  typ = {'double', 'complex', 'logical', 'sparse', 'complex sparse', ...
%!         'logical sparse', 'int8', 'int16', 'int32', 'int64', 'uint8', ...
%!         'uint16', 'uint32', 'uint64'};
%!
%!  cmplx = [2, 5];
%!  nlogical = [3, 6];
%!  ninteger = [7, 8, 9, 10, 11, 12, 13, 14];
%!  nsparse = [4, 5, 6];
%!  usesparse = true;
%!
%!  if (length (varargin) > 0 && islogical (varargin{1}))
%!    usesparse = varargin{1};
%!    varargin(1) = [];
%!  endif
%!
%!  for i = 1 : length(typ)
%!    m = min;
%!    if (length (varargin) > 0)
%!      args = varargin(1);
%!    else
%!      args = {};
%!    endif
%!
%!    if (any (nsparse == i))
%!      if (! usesparse)
%!        continue;
%!      endif
%!      if (ndims (m) > 2)
%!        sz = size (m);
%!        m = reshape (m, [sz(1), prod(sz (2:end))]);
%!      endif
%!      if (any (cmplx == i))
%!        m = sparse ((1 + 1i) * m);
%!      else
%!        m = sparse (m);
%!      endif
%!    else
%!      if (any (cmplx == i))
%!        m = (1 + 1i) * m;
%!      endif
%!    endif
%!    if (any (nlogical == i))
%!      m = cast (m, 'logical');
%!    endif
%!    if (any (ninteger == i))
%!      m = cast (m, typ{i});
%!    endif
%!
%!    y = feval (fn, m, args{:});
%!    if (!strcmp (class (y), class (m)) ||
%!         issparse (y) != issparse (m) ||
%!         any (cast (real (y), 'double')(:) !=
%!              feval (fn , cast (real (m), 'double'), args{:})(:)))
%!      error ('failed for type %s\n', typ{i});
%!    endif
%!  endfor
%! endfunction

%!shared m0, m1, m2, m3
%! m0 = [1:5];
%! m1 = reshape ([1 : 30], [5, 6]);
%! m2 = reshape ([1 : 30], [5, 1, 6]);
%! m3 = [];

%!test
%! __fntestfunc__('triu', m1);
%!test
%! __fntestfunc__ ('triu', m1, -1);
%!test
%! __fntestfunc__ ('triu', m1, 1);
%!test
%! __fntestfunc__('triu', m3);
%!test
%! __fntestfunc__ ('tril', m1);
%!test
%! __fntestfunc__ ('tril', m1, -1);
%!test
%! __fntestfunc__ ('tril', m1, 1);
%!test
%! __fntestfunc__('tril', m3);
%!test
%! __fntestfunc__ ('squeeze', m2);
%!test
%! __fntestfunc__ ('squeeze', m3);
%!test
%! __fntestfunc__ ('permute', m1, [2, 1]);
%!test
%! __fntestfunc__ ('permute', m2, false, [3, 1, 2]);
%!test
%! __fntestfunc__ ('permute', m3, [2, 1]);
%!test
%! __fntestfunc__ ('ipermute', m1, [2, 1]);
%!test
%! __fntestfunc__ ('ipermute', m2, false, [3, 1, 2]);
%!test
%! __fntestfunc__ ('ipermute', m3, [2, 1]);
%!test
%! __fntestfunc__ ('shiftdim', m2, 1);
%!test
%! __fntestfunc__ ('shiftdim', m2, false, -1);
%!test
%! __fntestfunc__ ('shiftdim', m3, 1);
%!test
%! __fntestfunc__ ('circshift', m2, 1);
%!test
%! __fntestfunc__ ('circshift', m2, [1, -1]);
%!test
%! __fntestfunc__ ('circshift', m3, 1);
%!test
%! __fntestfunc__ ('reshape', m2, [6, 5]);
%!test
%! __fntestfunc__ ('reshape', m3, [1, 0]);
%!test
%! __fntestfunc__ ('diag', m0);
%!test
%! __fntestfunc__ ('diag', m0, 1);
%!test
%! __fntestfunc__ ('diag', m0, -1);
%!test
%! __fntestfunc__ ('diag', m1);
%!test
%! __fntestfunc__ ('diag', m1, 1);
%!test
%! __fntestfunc__ ('diag', m1, -1);
%!test
%! __fntestfunc__ ('diag', m3);
%!test
%! __fntestfunc__ ('fliplr', m1);
%!test
%! __fntestfunc__ ('fliplr', m3);
%!test
%! __fntestfunc__ ('flipud', m1);
%!test
%! __fntestfunc__ ('flipud', m3);
%!test
%! __fntestfunc__ ('flipdim', m1, 2);
%!test
%! __fntestfunc__ ('flipdim', m3, 2);
%!test
%! __fntestfunc__ ('transpose', m1);
%!test
%! __fntestfunc__ ('transpose', m3);
%!test
%! __fntestfunc__ ('ctranspose', m1);
%!test
%! __fntestfunc__ ('ctranspose', m3);
%!test
%! __fntestfunc__ ('rot90', m1);
%!test
%! __fntestfunc__ ('rot90', m1, 2);
%!test
%! __fntestfunc__ ('rot90', m1, -1);
%!test
%! __fntestfunc__ ('rot90', m3);
%!test
%! __fntestfunc__ ('rotdim', m2, 1, [1, 2]);
%!test
%! __fntestfunc__ ('rotdim', m2, 2, [1, 2]);
%!test
%! __fntestfunc__ ('rotdim', m2, -1, [1, 2]);
%!test
%! __fntestfunc__ ('rotdim', m3, 1, [1, 2]);