[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