# HG changeset patch # User Rik # Date 1682528949 25200 # Node ID e242124f1240f174328a259a11bc471165166dcd # Parent f010a32986e414f10671eaa6c3692c17d49c48d6 Overhaul input validation of sparse() function. * sparse.cc (Fsparse): Decode input type and identify floating point inputs. If input is of single type, emit new warning "Octave:sparse:double-conversion". If input is neither floating point or logical, then call err_wrong_type_arg() for pretty error message. Rename temporary variable 'k' to "argidx" for clarity. New temporary variable "arg" to increase readability of code. Add FIXME note about unreachable code due to behavior of get_dimensions(). * sparse.cc (Fissparse): Turn off warning about double-conversion temporarily for test which has single input. * warning_ids.m: Add description for new warning ID "Octave:sparse:double-conversion". * mk-sparse-tst.sh: Redo BIST tests for sparse() construction. diff -r f010a32986e4 -r e242124f1240 libinterp/corefcn/sparse.cc --- a/libinterp/corefcn/sparse.cc Wed Apr 26 09:55:12 2023 -0700 +++ b/libinterp/corefcn/sparse.cc Wed Apr 26 10:09:09 2023 -0700 @@ -62,7 +62,9 @@ %!assert (issparse (1), false) %!assert (issparse (sparse (false)), true) %!assert (issparse (true), false) -%!assert (issparse (sparse (single ([1 2]))), true) +%!test +%! warning ("off", "Octave:sparse:double-conversion", "local"); +%! assert (issparse (sparse (single ([1 2]))), true); %!assert (issparse (single ([1, 2])), false) %!assert (issparse (sparse ([1+i, 2]')), true) %!assert (issparse ([1+i, 2]'), false) @@ -81,10 +83,10 @@ DEFUN (sparse, args, , doc: /* -*- texinfo -*- @deftypefn {} {@var{S} =} sparse (@var{A}) -@deftypefnx {} {@var{S} =} sparse (@var{i}, @var{j}, @var{sv}, @var{m}, @var{n}) +@deftypefnx {} {@var{S} =} sparse (@var{m}, @var{n}) @deftypefnx {} {@var{S} =} sparse (@var{i}, @var{j}, @var{sv}) -@deftypefnx {} {@var{S} =} sparse (@var{m}, @var{n}) -@deftypefnx {} {@var{S} =} sparse (@var{i}, @var{j}, @var{s}, @var{m}, @var{n}, "unique") +@deftypefnx {} {@var{S} =} sparse (@var{i}, @var{j}, @var{sv}, @var{m}, @var{n}) +@deftypefnx {} {@var{S} =} sparse (@var{i}, @var{j}, @var{sv}, @var{m}, @var{n}, "unique") @deftypefnx {} {@var{S} =} sparse (@var{i}, @var{j}, @var{sv}, @var{m}, @var{n}, @var{nzmax}) Create a sparse matrix from a full matrix @var{A} or row, column, value triplets. @@ -93,6 +95,9 @@ removing all zero values in the process. The matrix @var{A} should be of type logical or double. +If two inputs @var{m} (rows) and @var{n} (columns) are specified then create +an empty sparse matrix with the specified dimensions. + Given the integer index vectors @var{i} and @var{j}, and a 1-by-@code{nnz} vector of real or complex values @var{sv}, construct the sparse matrix @code{S(@var{i}(@var{k}),@var{j}(@var{k})) = @var{sv}(@var{k})} with overall @@ -111,7 +116,8 @@ If the option @qcode{"unique"} is given, and more than one value is specified at the same @var{i}, @var{j} indices, then only the last specified value will -be used. +be used. For completeness, the option @qcode{"sum"} can be given and will +be ignored as the default behavior is to sum values at repeated locations. @code{sparse (@var{m}, @var{n})} will create an empty @var{m}x@var{n} sparse matrix and is equivalent to @code{sparse ([], [], [], @var{m}, @var{n})} @@ -176,12 +182,18 @@ if (nargin == 1) { octave_value arg = args(0); - if (arg.islogical ()) + if (arg.isfloat ()) + { + if (arg.is_single_type ()) + warning_with_id ("Octave:sparse:double-conversion", + "sparse: input array cast to double"); + if (arg.iscomplex ()) + retval = arg.sparse_complex_matrix_value (); + else + retval = arg.sparse_matrix_value (); + } + else if (arg.islogical ()) retval = arg.sparse_bool_matrix_value (); - else if (arg.iscomplex ()) - retval = arg.sparse_complex_matrix_value (); - else if (arg.isnumeric ()) - retval = arg.sparse_matrix_value (); else err_wrong_type_arg ("sparse", arg); } @@ -192,6 +204,8 @@ get_dimensions (args(0), args(1), "sparse", m, n); + // FIXME: this code is never active because get_dimensions() + // replaces negative dimensions with 0. if (m < 0 || n < 0) error ("sparse: dimensions must be non-negative"); @@ -225,33 +239,42 @@ { get_dimensions (args(3), args(4), "sparse", m, n); + // FIXME: this code is never active because get_dimensions() + // replaces negative dimensions with 0. if (m < 0 || n < 0) error ("sparse: dimensions must be non-negative"); } - int k = 0; // index we're checking when index_vector throws + int argidx = 0; // index we're checking when index_vector throws try { idx_vector i = args(0).index_vector (); - k = 1; + argidx = 1; idx_vector j = args(1).index_vector (); - if (args(2).islogical ()) - retval = SparseBoolMatrix (args(2).bool_array_value (), i, j, - m, n, summation, nzmax); - else if (args(2).iscomplex ()) - retval = SparseComplexMatrix (args(2).complex_array_value(), - i, j, m, n, summation, nzmax); - else if (args(2).isnumeric ()) - retval = SparseMatrix (args(2).array_value (), i, j, - m, n, summation, nzmax); + octave_value arg = args(2); // temp var for code readability + if (arg.isfloat ()) + { + if (arg.is_single_type ()) + warning_with_id ("Octave:sparse:double-conversion", + "sparse: input array cast to double"); + if (arg.iscomplex ()) + retval = SparseComplexMatrix (arg.complex_array_value (), + i, j, m, n, summation, nzmax); + else + retval = SparseMatrix (arg.array_value (), + i, j, m, n, summation, nzmax); + } + else if (arg.islogical ()) + retval = SparseBoolMatrix (arg.bool_array_value (), + i, j, m, n, summation, nzmax); else - err_wrong_type_arg ("sparse", args(2)); + err_wrong_type_arg ("sparse", arg); } catch (index_exception& ie) { // Rethrow to allow more info to be reported later. - ie.set_pos_if_unset (2, k+1); + ie.set_pos_if_unset (2, argidx+1); throw; } } diff -r f010a32986e4 -r e242124f1240 scripts/help/warning_ids.m --- a/scripts/help/warning_ids.m Wed Apr 26 09:55:12 2023 -0700 +++ b/scripts/help/warning_ids.m Wed Apr 26 10:09:09 2023 -0700 @@ -410,6 +410,12 @@ ## string constant. ## By default, the @code{Octave:single-quote-string} warning is disabled. ## +## @item Octave:sparse:double-conversion +## If the @code{Octave:sparse:double-conversion} warning is enabled, a warning +## is printed when an implicit conversion from a full, single array occurs +## during the creation of a sparse array. +## By default, the @code{Octave:sparse:double-conversion} warning is enabled. +## ## @item Octave:sqrtm:SingularMatrix ## If the @code{Octave:sqrtm:SingularMatrix} warning is enabled, a warning is ## printed if the matrix square root function @code{sqrtm} is called with an @@ -459,5 +465,5 @@ endfunction -## Mark file as tested. No test needed for a documentation m-file. +## Mark file as tested. No tests needed for a documentation m-file. %!assert (1) diff -r f010a32986e4 -r e242124f1240 test/mk-sparse-tst.sh --- a/test/mk-sparse-tst.sh Wed Apr 26 09:55:12 2023 -0700 +++ b/test/mk-sparse-tst.sh Wed Apr 26 10:09:09 2023 -0700 @@ -174,6 +174,33 @@ gen_specific() { cat < sparse () +%!error sparse (1,2,3,4,5,6,7) +%!warning +%! warning ("on", "Octave:sparse:double-conversion", "local"); +%! s = sparse (single ([1 2])); +%!error +%! s = sparse (uint8 ([1 2])); +%% FIXME: negative dimensions are allowed and replaced with 0. +%% If this is fixed, re-instate these tests. +%!#error sparse (-1, 2) +%!#error sparse (1, -2) +%!error sparse (1,[2,3],[1,2,3]) +%!error sparse ([1,1],[1,1],[1,2],"foobar") +%% negative subscripts are disallowed +%!error sparse ([1,3],[1,-4],[3,5],2,2) +%!error sparse ([1,3],[1,-4],[3,5i],2,2) +%% FIXME: negative dimensions are allowed and replaced with 0. +%% If this is fixed, re-instate these tests. +%!#error sparse ([1,1],[1,1],[1,2], -1, 2) +%!#error sparse ([1,1],[1,1],[1,2], 1, -2) +%!warning +%! warning ("on", "Octave:sparse:double-conversion", "local"); +%! s = sparse ([1,1],[1,1], single ([1,2]), 2, 2); +%!error +%! s = sparse ([1,1],[1,1], uint8 ([1,2]), 2, 2); + %!test # segfault test from edd@debian.org %! n = 510; %! sparse (kron ((1:n)', ones (n,1)), kron (ones (n,1), (1:n)'), ones (n)); @@ -189,12 +216,6 @@ %#!error inv ( sparse ([0,0;0,1+i]) ); %#!error inv ( sparse ([0,0;0,0] ) ); -%% error handling in constructor -%!error sparse (1,[2,3],[1,2,3]) -%!error sparse ([1,1],[1,1],[1,2],3,3,"invalid") -%!error sparse ([1,3],[1,-4],[3,5],2,2) -%!error sparse ([1,3],[1,-4],[3,5i],2,2) -%!error sparse (-1,-1,1) EOF }