You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Thomas Neidhart <th...@gmail.com> on 2013/10/20 23:27:22 UTC

Re: svn commit: r1533990 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java

On 10/20/2013 11:24 PM, tn@apache.org wrote:
> Author: tn
> Date: Sun Oct 20 21:24:45 2013
> New Revision: 1533990
> 
> URL: http://svn.apache.org/r1533990
> Log:
> [MATH-1039] Avoid code duplication by calling logDensity itself.
> 
> Modified:
>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
> 
> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java?rev=1533990&r1=1533989&r2=1533990&view=diff
> ==============================================================================
> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java (original)
> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java Sun Oct 20 21:24:45 2013
> @@ -136,24 +136,8 @@ public class BetaDistribution extends Ab
>  
>      /** {@inheritDoc} */
>      public double density(double x) {
> -        recomputeZ();
> -        if (x < 0 || x > 1) {
> -            return 0;
> -        } else if (x == 0) {
> -            if (alpha < 1) {
> -                throw new NumberIsTooSmallException(LocalizedFormats.CANNOT_COMPUTE_BETA_DENSITY_AT_0_FOR_SOME_ALPHA, alpha, 1, false);
> -            }
> -            return 0;
> -        } else if (x == 1) {
> -            if (beta < 1) {
> -                throw new NumberIsTooSmallException(LocalizedFormats.CANNOT_COMPUTE_BETA_DENSITY_AT_1_FOR_SOME_BETA, beta, 1, false);
> -            }
> -            return 0;
> -        } else {
> -            double logX = FastMath.log(x);
> -            double log1mX = FastMath.log1p(-x);
> -            return FastMath.exp((alpha - 1) * logX + (beta - 1) * log1mX - z);
> -        }
> +        final double logDensity = logDensity(x);
> +        return logDensity == Double.NEGATIVE_INFINITY ? 0 : FastMath.exp(logDensity);
>      }
>  
>      /** {@inheritDoc} **/

I did this change for one class, but I propose to do this whereever
applicable to avoid code duplication in the distribution classes.

WDYT?

Thomas

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1533990 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java

Posted by Ted Dunning <te...@gmail.com>.
Thread issue.  Off topic for this thread.  No idea how this happened.


On Mon, Oct 21, 2013 at 3:25 PM, Phil Steitz <ph...@gmail.com> wrote:

> Was this maybe to the wrong thread, or is there a doco issue here?
>
> Phil
>
> > On Oct 20, 2013, at 10:42 PM, Ted Dunning <te...@gmail.com> wrote:
> >
> > This makes it somewhat harder to read the docs code which is where I
> read docs 90+% of the time.
> >
> > On the other hand my IDE will do the right thing if I ask it to.
> >
> > Sent from my iPhone
> >
> >> On Oct 20, 2013, at 14:27, Thomas Neidhart <th...@gmail.com>
> wrote:
> >>
> >>> On 10/20/2013 11:24 PM, tn@apache.org wrote:
> >>> Author: tn
> >>> Date: Sun Oct 20 21:24:45 2013
> >>> New Revision: 1533990
> >>>
> >>> URL: http://svn.apache.org/r1533990
> >>> Log:
> >>> [MATH-1039] Avoid code duplication by calling logDensity itself.
> >>>
> >>> Modified:
> >>>
> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
> >>>
> >>> Modified:
> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
> >>> URL:
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java?rev=1533990&r1=1533989&r2=1533990&view=diff
> >>>
> ==============================================================================
> >>> ---
> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
> (original)
> >>> +++
> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
> Sun Oct 20 21:24:45 2013
> >>> @@ -136,24 +136,8 @@ public class BetaDistribution extends Ab
> >>>
> >>>    /** {@inheritDoc} */
> >>>    public double density(double x) {
> >>> -        recomputeZ();
> >>> -        if (x < 0 || x > 1) {
> >>> -            return 0;
> >>> -        } else if (x == 0) {
> >>> -            if (alpha < 1) {
> >>> -                throw new
> NumberIsTooSmallException(LocalizedFormats.CANNOT_COMPUTE_BETA_DENSITY_AT_0_FOR_SOME_ALPHA,
> alpha, 1, false);
> >>> -            }
> >>> -            return 0;
> >>> -        } else if (x == 1) {
> >>> -            if (beta < 1) {
> >>> -                throw new
> NumberIsTooSmallException(LocalizedFormats.CANNOT_COMPUTE_BETA_DENSITY_AT_1_FOR_SOME_BETA,
> beta, 1, false);
> >>> -            }
> >>> -            return 0;
> >>> -        } else {
> >>> -            double logX = FastMath.log(x);
> >>> -            double log1mX = FastMath.log1p(-x);
> >>> -            return FastMath.exp((alpha - 1) * logX + (beta - 1) *
> log1mX - z);
> >>> -        }
> >>> +        final double logDensity = logDensity(x);
> >>> +        return logDensity == Double.NEGATIVE_INFINITY ? 0 :
> FastMath.exp(logDensity);
> >>>    }
> >>>
> >>>    /** {@inheritDoc} **/
> >>
> >> I did this change for one class, but I propose to do this whereever
> >> applicable to avoid code duplication in the distribution classes.
> >>
> >> WDYT?
> >>
> >> Thomas
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Re: svn commit: r1533990 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java

Posted by Phil Steitz <ph...@gmail.com>.
Was this maybe to the wrong thread, or is there a doco issue here?

Phil

> On Oct 20, 2013, at 10:42 PM, Ted Dunning <te...@gmail.com> wrote:
> 
> This makes it somewhat harder to read the docs code which is where I read docs 90+% of the time.  
> 
> On the other hand my IDE will do the right thing if I ask it to. 
> 
> Sent from my iPhone
> 
>> On Oct 20, 2013, at 14:27, Thomas Neidhart <th...@gmail.com> wrote:
>> 
>>> On 10/20/2013 11:24 PM, tn@apache.org wrote:
>>> Author: tn
>>> Date: Sun Oct 20 21:24:45 2013
>>> New Revision: 1533990
>>> 
>>> URL: http://svn.apache.org/r1533990
>>> Log:
>>> [MATH-1039] Avoid code duplication by calling logDensity itself.
>>> 
>>> Modified:
>>>   commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
>>> 
>>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
>>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java?rev=1533990&r1=1533989&r2=1533990&view=diff
>>> ==============================================================================
>>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java (original)
>>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java Sun Oct 20 21:24:45 2013
>>> @@ -136,24 +136,8 @@ public class BetaDistribution extends Ab
>>> 
>>>    /** {@inheritDoc} */
>>>    public double density(double x) {
>>> -        recomputeZ();
>>> -        if (x < 0 || x > 1) {
>>> -            return 0;
>>> -        } else if (x == 0) {
>>> -            if (alpha < 1) {
>>> -                throw new NumberIsTooSmallException(LocalizedFormats.CANNOT_COMPUTE_BETA_DENSITY_AT_0_FOR_SOME_ALPHA, alpha, 1, false);
>>> -            }
>>> -            return 0;
>>> -        } else if (x == 1) {
>>> -            if (beta < 1) {
>>> -                throw new NumberIsTooSmallException(LocalizedFormats.CANNOT_COMPUTE_BETA_DENSITY_AT_1_FOR_SOME_BETA, beta, 1, false);
>>> -            }
>>> -            return 0;
>>> -        } else {
>>> -            double logX = FastMath.log(x);
>>> -            double log1mX = FastMath.log1p(-x);
>>> -            return FastMath.exp((alpha - 1) * logX + (beta - 1) * log1mX - z);
>>> -        }
>>> +        final double logDensity = logDensity(x);
>>> +        return logDensity == Double.NEGATIVE_INFINITY ? 0 : FastMath.exp(logDensity);
>>>    }
>>> 
>>>    /** {@inheritDoc} **/
>> 
>> I did this change for one class, but I propose to do this whereever
>> applicable to avoid code duplication in the distribution classes.
>> 
>> WDYT?
>> 
>> Thomas
>> 
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1533990 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java

Posted by Ted Dunning <te...@gmail.com>.
This makes it somewhat harder to read the docs code which is where I read docs 90+% of the time.  

On the other hand my IDE will do the right thing if I ask it to. 

Sent from my iPhone

On Oct 20, 2013, at 14:27, Thomas Neidhart <th...@gmail.com> wrote:

> On 10/20/2013 11:24 PM, tn@apache.org wrote:
>> Author: tn
>> Date: Sun Oct 20 21:24:45 2013
>> New Revision: 1533990
>> 
>> URL: http://svn.apache.org/r1533990
>> Log:
>> [MATH-1039] Avoid code duplication by calling logDensity itself.
>> 
>> Modified:
>>    commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
>> 
>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java?rev=1533990&r1=1533989&r2=1533990&view=diff
>> ==============================================================================
>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java (original)
>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java Sun Oct 20 21:24:45 2013
>> @@ -136,24 +136,8 @@ public class BetaDistribution extends Ab
>> 
>>     /** {@inheritDoc} */
>>     public double density(double x) {
>> -        recomputeZ();
>> -        if (x < 0 || x > 1) {
>> -            return 0;
>> -        } else if (x == 0) {
>> -            if (alpha < 1) {
>> -                throw new NumberIsTooSmallException(LocalizedFormats.CANNOT_COMPUTE_BETA_DENSITY_AT_0_FOR_SOME_ALPHA, alpha, 1, false);
>> -            }
>> -            return 0;
>> -        } else if (x == 1) {
>> -            if (beta < 1) {
>> -                throw new NumberIsTooSmallException(LocalizedFormats.CANNOT_COMPUTE_BETA_DENSITY_AT_1_FOR_SOME_BETA, beta, 1, false);
>> -            }
>> -            return 0;
>> -        } else {
>> -            double logX = FastMath.log(x);
>> -            double log1mX = FastMath.log1p(-x);
>> -            return FastMath.exp((alpha - 1) * logX + (beta - 1) * log1mX - z);
>> -        }
>> +        final double logDensity = logDensity(x);
>> +        return logDensity == Double.NEGATIVE_INFINITY ? 0 : FastMath.exp(logDensity);
>>     }
>> 
>>     /** {@inheritDoc} **/
> 
> I did this change for one class, but I propose to do this whereever
> applicable to avoid code duplication in the distribution classes.
> 
> WDYT?
> 
> Thomas
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: svn commit: r1533990 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java

Posted by Thomas Neidhart <th...@gmail.com>.
Ok np, there are other cases where the code could be simplified, I will do
the necessary changes.

Thomas


On Mon, Oct 21, 2013 at 12:05 AM, Phil Steitz <ph...@gmail.com> wrote:

> On 10/20/13 2:27 PM, Thomas Neidhart wrote:
> > On 10/20/2013 11:24 PM, tn@apache.org wrote:
> >> Author: tn
> >> Date: Sun Oct 20 21:24:45 2013
> >> New Revision: 1533990
> >>
> >> URL: http://svn.apache.org/r1533990
> >> Log:
> >> [MATH-1039] Avoid code duplication by calling logDensity itself.
> >>
> >> Modified:
> >>
> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
> >>
> >> Modified:
> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
> >> URL:
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java?rev=1533990&r1=1533989&r2=1533990&view=diff
> >>
> ==============================================================================
> >> ---
> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
> (original)
> >> +++
> commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
> Sun Oct 20 21:24:45 2013
> >> @@ -136,24 +136,8 @@ public class BetaDistribution extends Ab
> >>
> >>      /** {@inheritDoc} */
> >>      public double density(double x) {
> >> -        recomputeZ();
> >> -        if (x < 0 || x > 1) {
> >> -            return 0;
> >> -        } else if (x == 0) {
> >> -            if (alpha < 1) {
> >> -                throw new
> NumberIsTooSmallException(LocalizedFormats.CANNOT_COMPUTE_BETA_DENSITY_AT_0_FOR_SOME_ALPHA,
> alpha, 1, false);
> >> -            }
> >> -            return 0;
> >> -        } else if (x == 1) {
> >> -            if (beta < 1) {
> >> -                throw new
> NumberIsTooSmallException(LocalizedFormats.CANNOT_COMPUTE_BETA_DENSITY_AT_1_FOR_SOME_BETA,
> beta, 1, false);
> >> -            }
> >> -            return 0;
> >> -        } else {
> >> -            double logX = FastMath.log(x);
> >> -            double log1mX = FastMath.log1p(-x);
> >> -            return FastMath.exp((alpha - 1) * logX + (beta - 1) *
> log1mX - z);
> >> -        }
> >> +        final double logDensity = logDensity(x);
> >> +        return logDensity == Double.NEGATIVE_INFINITY ? 0 :
> FastMath.exp(logDensity);
> >>      }
> >>
> >>      /** {@inheritDoc} **/
> > I did this change for one class, but I propose to do this whereever
> > applicable to avoid code duplication in the distribution classes.
> >
> > WDYT?
>
> Nice catch.  By all means, when code is actually duplicated.  Sorry
> I missed this.
>
> Phil
> >
> > Thomas
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Re: svn commit: r1533990 - /commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java

Posted by Phil Steitz <ph...@gmail.com>.
On 10/20/13 2:27 PM, Thomas Neidhart wrote:
> On 10/20/2013 11:24 PM, tn@apache.org wrote:
>> Author: tn
>> Date: Sun Oct 20 21:24:45 2013
>> New Revision: 1533990
>>
>> URL: http://svn.apache.org/r1533990
>> Log:
>> [MATH-1039] Avoid code duplication by calling logDensity itself.
>>
>> Modified:
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
>>
>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java?rev=1533990&r1=1533989&r2=1533990&view=diff
>> ==============================================================================
>> --- commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java (original)
>> +++ commons/proper/math/trunk/src/main/java/org/apache/commons/math3/distribution/BetaDistribution.java Sun Oct 20 21:24:45 2013
>> @@ -136,24 +136,8 @@ public class BetaDistribution extends Ab
>>  
>>      /** {@inheritDoc} */
>>      public double density(double x) {
>> -        recomputeZ();
>> -        if (x < 0 || x > 1) {
>> -            return 0;
>> -        } else if (x == 0) {
>> -            if (alpha < 1) {
>> -                throw new NumberIsTooSmallException(LocalizedFormats.CANNOT_COMPUTE_BETA_DENSITY_AT_0_FOR_SOME_ALPHA, alpha, 1, false);
>> -            }
>> -            return 0;
>> -        } else if (x == 1) {
>> -            if (beta < 1) {
>> -                throw new NumberIsTooSmallException(LocalizedFormats.CANNOT_COMPUTE_BETA_DENSITY_AT_1_FOR_SOME_BETA, beta, 1, false);
>> -            }
>> -            return 0;
>> -        } else {
>> -            double logX = FastMath.log(x);
>> -            double log1mX = FastMath.log1p(-x);
>> -            return FastMath.exp((alpha - 1) * logX + (beta - 1) * log1mX - z);
>> -        }
>> +        final double logDensity = logDensity(x);
>> +        return logDensity == Double.NEGATIVE_INFINITY ? 0 : FastMath.exp(logDensity);
>>      }
>>  
>>      /** {@inheritDoc} **/
> I did this change for one class, but I propose to do this whereever
> applicable to avoid code duplication in the distribution classes.
>
> WDYT?

Nice catch.  By all means, when code is actually duplicated.  Sorry
I missed this.

Phil
>
> Thomas
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org