octave-maintainers
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: problem with sum (...)


From: John W. Eaton
Subject: Re: problem with sum (...)
Date: Tue, 06 Nov 2007 17:44:10 -0500

On  6-Nov-2007, David Bateman wrote:

| John W. Eaton wrote:
| > On  5-Nov-2007, David Bateman wrote:
| > 
| > | In any case consider the attached patch that adds this
| > | functionality to "sum"..
| > 
| > I applied the patch for OPERATORS/op-bm-bm.cc but not the other part
| > because it doesn't seem to implement the correctly functionality.
| > If I'm understanding correctly, your patch computes the sum using
| > doubles internally, then casts the result back to the original type.
| > That's not quite the same as doing the sum in the native type, which
| > is what the Matlab docs say they are doing.  For example, with the
| > saturation rules for integer arithmetic, Matlab computes
| > 
| >   sum (int8 ([127, 10, -20]), 'native')
| >   == (127 + 10) + -20
| >   == 127 + -20
| >   == 107
| 
| Not that I don't believe you, but I checked and with MatlabR2007a this
| returns 107 as you stated.. Damn.
| 
| > 
| > but with your patch, we get 117 because the saturation limit is only
| > applied at the end instead of after each operation.
| 
| Ok, then what about the attached..
| 
| > 
| > Note that with 'native' it is critical to sum the elements in a
| > specific order.  Ugh.
| 
| The attached patch also adds a test for this
| 
| > 
| > | Perhaps the other data reduction operators
| > | should also include this functionality?
| > 
| > Hmm.  Maybe only if it is needed for compatibility.
| 
| Ok, then lets not add it to the other functions.

I'd say apply the patch.

In the future, I think we will want to look for any code that uses
if/elseif switches like this and have them dispatch via octave_value
member functions instead.  We can look at that for 3.1 or later.

Thanks,

jwe


reply via email to

[Prev in Thread] Current Thread [Next in Thread]