# HG changeset patch # User Rik # Date 1548193545 28800 # Node ID 5ab12183280d1b4693b448a4aaa3bb29a79e2272 # Parent cc0d942d0e20dee755c065ff85f470b47b931210 dot.cc: Call Fsum directly rather than using feval. * dot.cc (Fdot): #include "builtin-defun-decls.h" for prototypes of Octave functions. Update docstring to warn about incorrect results when using integer arrays. Add FIXME note to else branch which calculates integer results suggesting a warning or error might be appropriate. Replace feval call with direct call to Fsum. Update BIST tests to use commas in matrix definitions. Add BIST test for an int8 object which clearly calculates the wrong result. diff -r cc0d942d0e20 -r 5ab12183280d libinterp/corefcn/dot.cc --- a/libinterp/corefcn/dot.cc Tue Jan 22 14:34:08 2019 +0100 +++ b/libinterp/corefcn/dot.cc Tue Jan 22 13:45:45 2019 -0800 @@ -26,8 +26,10 @@ #include "lo-blas-proto.h" #include "mx-base.h" + +#include "builtin-defun-decls.h" +#include "defun.h" #include "error.h" -#include "defun.h" #include "parse.h" static void @@ -75,11 +77,12 @@ If the optional argument @var{dim} is given, calculate the dot products along this dimension. -This is equivalent to -@code{sum (conj (@var{X}) .* @var{Y}, @var{dim})}, -but avoids forming a temporary array and is faster. When @var{X} and -@var{Y} are column vectors, the result is equivalent to -@code{@var{X}' * @var{Y}}. +Implementation Note: This is equivalent to +@code{sum (conj (@var{X}) .* @var{Y}, @var{dim})}, but avoids forming a +temporary array and is faster. When @var{X} and @var{Y} are column vectors, +the result is equivalent to @code{@var{X}' * @var{Y}}. Although, @code{dot} +is defined for integer arrays, the output may differ from the expected result +due to the limited range of integer objects. @seealso{cross, divergence} @end deftypefn */) { @@ -177,11 +180,16 @@ else { // Non-optimized evaluation. + // FIXME: This may *not* do what the user expects. + // It might be more useful to issue a warning, or even an error, instead + // of calculating possibly garbage results. + // Think of the dot product of two int8 vectors where the multiplications + // exceed intmax. octave_value_list tmp; tmp(1) = dim + 1; tmp(0) = do_binary_op (octave_value::op_el_mul, argx, argy); - tmp = octave::feval ("sum", tmp, 1); + tmp = Fsum (tmp, 1); if (! tmp.empty ()) retval = tmp(0); } @@ -204,17 +212,23 @@ %! assert (dot (single (x), single (x)), single ([4, 20])); %!test -%! x = int8 ([1 2]); -%! y = int8 ([2 3]); +%! x = int8 ([1, 2]); +%! y = int8 ([2, 3]); %! assert (dot (x, y), 8); %!test -%! x = int8 ([1 2; 3 4]); -%! y = int8 ([5 6; 7 8]); +%! x = int8 ([1, 2; 3, 4]); +%! y = int8 ([5, 6; 7, 8]); %! assert (dot (x, y), [26 44]); %! assert (dot (x, y, 2), [17; 53]); %! assert (dot (x, y, 3), [5 12; 21 32]); +## This is, perhaps, surprising. Integer maximums and saturation mechanics +## prevent accurate value from being calculated. +%!test +%! x = int8 ([127]); +%! assert (dot (x, x), 127); + ## Test input validation %!error dot () %!error dot (1)