Mercurial > octave

changeset 26599:5ab12183280d

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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.
author Rik <rik@octave.org>
date Tue, 22 Jan 2019 13:45:45 -0800
parents cc0d942d0e20
children 10b824cf2b18
files libinterp/corefcn/dot.cc
diffstat 1 files changed, 25 insertions(+), 11 deletions(-) [+]
line wrap: on
line diff
--- 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)