You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by "Mark R. Diggory" <md...@latte.harvard.edu> on 2004/07/18 18:22:15 UTC
[math] Re: cvs commit: jakarta-commons/math/src/java/org/apache/commons/math/stat/univariate/moment
GeometricMean.java
Hey Phil,
I notice your moving much of the extended functionality out of the
Univariates and into a private field instead. I'm just curious what your
logic is behind it? I had considered wrapping vs extending initially
when I was building the implementation. Now that I look back, my reasons
for going with extending were more to take advantage of the already
preexisting "protected" fields in the implementation of the underlying
Univariate. Now that I see your changes, I think what your wrapping is
more kosher in terms of encapsulation underlying implementation of the
wrapped statistic.
While the difference between these approaches is minimal in the front
end Univariates like Mean, Var, Median, StandardDev and Geomean, I
believe the extended hierarchy of FirstMoment -> SecondMoment ->
ThirdMoment -> FourthMoment takes more advantage of this extended field
reuse. I'm not so sure if these should be approached as wrappers too?
They do make significant reuse of the internal state of the underlying
moment. What do you think? Would these be problematic to turn into
wrapped implementations too?
-Mark
psteitz@apache.org wrote:
> psteitz 2004/07/17 21:37:08
>
> Modified: math/src/java/org/apache/commons/math/stat/univariate/moment
> GeometricMean.java
> Log:
> Changed implementation to wrap, rather than extend SumOfLogs.
>
> Revision Changes Path
> 1.22 +30 -20 jakarta-commons/math/src/java/org/apache/commons/math/stat/univariate/moment/GeometricMean.java
>
> Index: GeometricMean.java
> ===================================================================
> RCS file: /home/cvs/jakarta-commons/math/src/java/org/apache/commons/math/stat/univariate/moment/GeometricMean.java,v
> retrieving revision 1.21
> retrieving revision 1.22
> diff -u -r1.21 -r1.22
> --- GeometricMean.java 4 Jul 2004 09:02:36 -0000 1.21
> +++ GeometricMean.java 18 Jul 2004 04:37:08 -0000 1.22
> @@ -15,20 +15,20 @@
> */
> package org.apache.commons.math.stat.univariate.moment;
>
> -import java.io.Serializable;
> -
> +import org.apache.commons.math.stat.univariate.AbstractStorelessUnivariateStatistic;
> import org.apache.commons.math.stat.univariate.summary.SumOfLogs;
>
> /**
> * Returns the <a href="http://www.xycoon.com/geometric_mean.htm">
> * geometric mean </a> of the available values.
> * <p>
> - * Uses {@link SumOfLogs} superclass to compute sum of logs and returns
> + * Uses a {@link SumOfLogs} instance to compute sum of logs and returns
> * <code> exp( 1/n (sum of logs) ).</code> Therefore,
> * <ul>
> * <li>If any of values are < 0, the result is <code>NaN.</code></li>
> - * <li>If all values are non-negative and less than <code>Double.POSITIVE_INFINITY</code>,
> - * but at least one value is 0, the result is <code>0.</code></li>
> + * <li>If all values are non-negative and less than
> + * <code>Double.POSITIVE_INFINITY</code>, but at least one value is 0, the
> + * result is <code>0.</code></li>
> * <li>If both <code>Double.POSITIVE_INFINITY</code> and
> * <code>Double.NEGATIVE_INFINITY</code> are among the values, the result is
> * <code>NaN.</code></li>
> @@ -42,28 +42,34 @@
> *
> * @version $Revision$ $Date$
> */
> -public class GeometricMean extends SumOfLogs implements Serializable{
> +public class GeometricMean extends AbstractStorelessUnivariateStatistic {
>
> /** Serializable version identifier */
> static final long serialVersionUID = -8178734905303459453L;
> -
> - /**Number of values that have been added */
> - protected long n = 0;
> +
> + /** Wrapped SumOfLogs instance */
> + private SumOfLogs sumOfLogs;
>
> /**
> + * Create a GeometricMean instance
> + */
> + public GeometricMean() {
> + sumOfLogs = new SumOfLogs();
> + }
> +
> + /**
> * @see org.apache.commons.math.stat.univariate.StorelessUnivariateStatistic#increment(double)
> */
> public void increment(final double d) {
> - n++;
> - super.increment(d);
> + sumOfLogs.increment(d);
> }
>
> /**
> * @see org.apache.commons.math.stat.univariate.StorelessUnivariateStatistic#getResult()
> */
> public double getResult() {
> - if (n > 0) {
> - return Math.exp(super.getResult() / (double) n);
> + if (sumOfLogs.getN() > 0) {
> + return Math.exp(sumOfLogs.getResult() / (double) sumOfLogs.getN());
> } else {
> return Double.NaN;
> }
> @@ -73,8 +79,7 @@
> * @see org.apache.commons.math.stat.univariate.StorelessUnivariateStatistic#clear()
> */
> public void clear() {
> - super.clear();
> - n = 0;
> + sumOfLogs.clear();
> }
>
> /**
> @@ -94,11 +99,16 @@
> * index parameters are not valid
> */
> public double evaluate(
> - final double[] values,
> - final int begin,
> - final int length) {
> + final double[] values, final int begin, final int length) {
> return Math.exp(
> - super.evaluate(values, begin, length) / (double) length);
> + sumOfLogs.evaluate(values, begin, length) / (double) length);
> + }
> +
> + /**
> + * @see org.apache.commons.math.stat.univariate.StorelessUnivariateStatistic#getN()
> + */
> + public long getN() {
> + return sumOfLogs.getN();
> }
>
> }
>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
--
Mark Diggory
Software Developer
Harvard MIT Data Center
http://www.hmdc.harvard.edu
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [math] Composition vs. Inheritance for moment statistics
Posted by Phil Steitz <ph...@steitz.com>.
Mark R. Diggory wrote:
> Hey Phil,
>
> I notice your moving much of the extended functionality out of the
> Univariates and into a private field instead. I'm just curious what your
> logic is behind it? I had considered wrapping vs extending initially
> when I was building the implementation. Now that I look back, my reasons
> for going with extending were more to take advantage of the already
> preexisting "protected" fields in the implementation of the underlying
> Univariate. Now that I see your changes, I think what your wrapping is
> more kosher in terms of encapsulation underlying implementation of the
> wrapped statistic.
I think we are agreeing here. The ones that I changed were
StandardDeviation (wrapping, rather than extending Variance) and
GeometricMean (wrapping, rather than extending SumOfLogs). The reason for
this, as you point out, is that extending (in these cases) binds the
implementations unecessarily. In the two cases that I changed, what is
really going on is just delegation, so composition is better (IMHO).
>
> While the difference between these approaches is minimal in the front
> end Univariates like Mean, Var, Median, StandardDev and Geomean, I
> believe the extended hierarchy of FirstMoment -> SecondMoment ->
> ThirdMoment -> FourthMoment takes more advantage of this extended field
> reuse. I'm not so sure if these should be approached as wrappers too?
> They do make significant reuse of the internal state of the underlying
> moment. What do you think? Would these be problematic to turn into
> wrapped implementations too?
I thought about those and they could be changed to nest instead of extend
one another; but as you point out there is significant use of common
fields in these classes and it would be a fair amount of work to pull them
apart. In addition, some efficiency might be lost by pulling them apart.
This is beyond my JVM knowledge; but it seems that it might be more
expensive to create the resulting "Chinese Box" of nested moment instances
for, e.g. FourthMoment, and dereference all of the fields attached to the
inner moments when doing computations, rather than just accessing the
protected fields directly. This is especially true of the lower order
moment values themselves, which are now stored as doubles and would have
to be accessed via getResult() calls to the nested moments were we to
change to composition.
Phil
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org