Re: Bug in geomean()

[Jordi Gutiérrez Hermoso]

On 1 October 2012 11:25, kyfsympsn <address@hidden> wrote:
> Thanks Jordi,
>
>
>> I think it's a good habit to inspect the source code if you want to
>> know how something is being done.
>
> Yeah, sorry... making the switch from Matlab, so I'm not yet in the habit of
> "looking under the hood" at the source code :-)
>
>
>> Can you provide a patch? It should take into account the cases of
>> complex, negative, or zero entries.
>
> the basic fix is to change that line 116 from:
>    y = prod (x, dim) .^ (1/n);
> to:
>    y = exp (sum(log(x), dim) ./n);
>
> --zero entries in x will correctly return 0
>
> --Using the current method negative entries return a complex result, however
> I don't think it makes sense given the definition and uses of the geometric
> mean (wikipedia also says the geometric mean should only be performed on
> positive numbers).

> Using my suggested modification, negative entries will fail (as they
> do in matlab), although it would be useful to the user to say why,
> so i guess line 116 should become:
>
> if (sum(sum(x<0))==0)
>    y = exp (sum(log(x), dim) ./n);
> else
>     error ("mean: X must not contain any negative values");
> end

I have implemented a slight variation of this for Octave (endif
instead of plain end, spaces after function calls to distinguish them
from indexing calls), and added a test for it:

    http://hg.savannah.gnu.org/hgweb/octave/rev/930117c97760

I gave you no acknowledgement, since you seem to prefer
semi-anonymity.

The current version of the NaN package already seems to have a fix for
this, and has been untouched for the past 17 months, so I'm surprised
you saw this problem:

    
http://octave.svn.sourceforge.net/viewvc/octave/trunk/octave-forge/extra/NaN/inst/geomean.m?revision=8223&view=markup

Thanks,
- Jordi G. H.