You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Phil Steitz <ph...@gmail.com> on 2009/04/05 20:30:10 UTC
Re: svn commit: r762116 - in /commons/proper/math/trunk/src/java/org/apache/commons/math:
./ analysis/polynomials/ analysis/solvers/ estimation/ ode/ ode/nonstiff/
ode/sampling/ optimization/ random/ stat/ stat/descriptive/ stat/descriptive/moment/
s
sebb wrote:
> On 05/04/2009, Phil Steitz <ph...@gmail.com> wrote:
>
>> sebb@apache.org wrote:
>>
>> We need to be careful with removing all of these casts. They were inserted
>> earlier to eliminate some errors resulting from int/int computations in
>> larger expressions resulting in incorrect values. Could be later JDKs
>> handle all of this seamlessly, but I would prefer to be safe rather than
>> sorry here and leave the explicit casts from int/long to double alone.
>>
>
> Are there any tests that expose these errors?
>
The ones we discovered, yes. IIRC, they were all of the form long/long
or long*long embedded as below in a larger floating point computations.
> Seems to me that if an apparently unnecessary cast is included in the
> code, this should also be documented so it is clear why it is there.
>
Or you could look at it as the cast itself as clarifying intent ;)
Phil
>> Phil
>>
>>
>>> Modified:
>>>
>> commons/proper/math/trunk/src/java/org/apache/commons/math/stat/regression/SimpleRegression.java
>>
>>> URL:
>>>
>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/java/org/apache/commons/math/stat/regression/SimpleRegression.java?rev=762116&r1=762115&r2=762116&view=diff
>>
>> ==============================================================================
>>
>>> ---
>>>
>> commons/proper/math/trunk/src/java/org/apache/commons/math/stat/regression/SimpleRegression.java
>> (original)
>>
>>> +++
>>>
>> commons/proper/math/trunk/src/java/org/apache/commons/math/stat/regression/SimpleRegression.java
>> Sun Apr 5 16:48:53 2009
>>
>>> @@ -124,11 +124,11 @@
>>> } else {
>>> double dx = x - xbar;
>>> double dy = y - ybar;
>>> - sumXX += dx * dx * (double) n / (double) (n + 1.0);
>>> - sumYY += dy * dy * (double) n / (double) (n + 1.0);
>>> - sumXY += dx * dy * (double) n / (double) (n + 1.0);
>>> - xbar += dx / (double) (n + 1.0);
>>> - ybar += dy / (double) (n + 1.0);
>>> + sumXX += dx * dx * n / (n + 1.0);
>>> + sumYY += dy * dy * n / (n + 1.0);
>>> + sumXY += dx * dy * n / (n + 1.0);
>>> + xbar += dx / (n + 1.0);
>>> + ybar += dy / (n + 1.0);
>>> }
>>> sumX += x;
>>> sumY += y;
>>> @@ -157,11 +157,11 @@
>>> if (n > 0) {
>>> double dx = x - xbar;
>>> double dy = y - ybar;
>>> - sumXX -= dx * dx * (double) n / (double) (n - 1.0);
>>> - sumYY -= dy * dy * (double) n / (double) (n - 1.0);
>>> - sumXY -= dx * dy * (double) n / (double) (n - 1.0);
>>> - xbar -= dx / (double) (n - 1.0);
>>> - ybar -= dy / (double) (n - 1.0);
>>> + sumXX -= dx * dx * n / (n - 1.0);
>>> + sumYY -= dy * dy * n / (n - 1.0);
>>> + sumXY -= dx * dy * n / (n - 1.0);
>>> + xbar -= dx / (n - 1.0);
>>> + ybar -= dy / (n - 1.0);
>>> sumX -= x;
>>> sumY -= y;
>>> n--;
>>> @@ -410,7 +410,7 @@
>>> if (n < 3) {
>>> return Double.NaN;
>>> }
>>> - return getSumSquaredErrors() / (double) (n - 2);
>>> + return getSumSquaredErrors() / (n - 2);
>>> }
>>> /**
>>> @@ -468,7 +468,7 @@
>>> */
>>> public double getInterceptStdErr() {
>>> return Math.sqrt(
>>> - getMeanSquareError() * ((1d / (double) n) + (xbar * xbar) /
>>>
>> sumXX));
>>
>>> + getMeanSquareError() * ((1d / n) + (xbar * xbar) / sumXX));
>>> }
>>> /**
>>> @@ -589,7 +589,7 @@
>>> * @return the intercept of the regression line
>>> */
>>> private double getIntercept(double slope) {
>>> - return (sumY - slope * sumX) / ((double) n);
>>> + return (sumY - slope * sumX) / (n);
>>> }
>>> /**
>>>
>>> Modified:
>>>
>> commons/proper/math/trunk/src/java/org/apache/commons/math/util/MathUtils.java
>>
>>> URL:
>>>
>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/java/org/apache/commons/math/util/MathUtils.java?rev=762116&r1=762115&r2=762116&view=diff
>>
>> ==============================================================================
>>
>>> ---
>>>
>> commons/proper/math/trunk/src/java/org/apache/commons/math/util/MathUtils.java
>> (original)
>>
>>> +++
>>>
>> commons/proper/math/trunk/src/java/org/apache/commons/math/util/MathUtils.java
>> Sun Apr 5 16:48:53 2009
>>
>>> @@ -307,7 +307,7 @@
>>> return 0;
>>> }
>>> if ((k == 1) || (k == n - 1)) {
>>> - return Math.log((double) n);
>>> + return Math.log(n);
>>> }
>>> /*
>>> @@ -337,12 +337,12 @@
>>> // n!/(n-k)!
>>> for (int i = n - k + 1; i <= n; i++) {
>>> - logSum += Math.log((double) i);
>>> + logSum += Math.log(i);
>>> }
>>> // divide by k!
>>> for (int i = 2; i <= k; i++) {
>>> - logSum -= Math.log((double) i);
>>> + logSum -= Math.log(i);
>>> }
>>> return logSum; @@ -523,7 +523,7 @@
>>> }
>>> double logSum = 0;
>>> for (int i = 2; i <= n; i++) {
>>> - logSum += Math.log((double)i);
>>> + logSum += Math.log(i);
>>> }
>>> return logSum;
>>> }
>>>
>>> Modified:
>>>
>> commons/proper/math/trunk/src/java/org/apache/commons/math/util/TransformerMap.java
>>
>>> URL:
>>>
>> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/java/org/apache/commons/math/util/TransformerMap.java?rev=762116&r1=762115&r2=762116&view=diff
>>
>> ==============================================================================
>>
>>> ---
>>>
>> commons/proper/math/trunk/src/java/org/apache/commons/math/util/TransformerMap.java
>> (original)
>>
>>> +++
>>>
>> commons/proper/math/trunk/src/java/org/apache/commons/math/util/TransformerMap.java
>> Sun Apr 5 16:48:53 2009
>>
>>> @@ -79,7 +79,7 @@
>>> * @return the mapped NumberTransformer or null.
>>> */
>>> public NumberTransformer getTransformer(Class<?> key) {
>>> - return (NumberTransformer) map.get(key);
>>> + return map.get(key);
>>> }
>>> /**
>>>
>>>
>>>
>>>
>>>
>> ---------------------------------------------------------------------
>> 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: r762116 - in /commons/proper/math/trunk/src/java/org/apache/commons/math:
./ analysis/polynomials/ analysis/solvers/ estimation/ ode/ ode/nonstiff/
ode/sampling/ optimization/ random/ stat/ stat/descriptive/
stat/descriptive/moment/ s
Posted by sebb <se...@gmail.com>.
On 05/04/2009, Phil Steitz <ph...@gmail.com> wrote:
> sebb wrote:
>
> > On 05/04/2009, Phil Steitz <ph...@gmail.com> wrote:
> >
> >
> > > sebb@apache.org wrote:
> > >
> > > We need to be careful with removing all of these casts. They were
> inserted
> > > earlier to eliminate some errors resulting from int/int computations in
> > > larger expressions resulting in incorrect values. Could be later JDKs
> > > handle all of this seamlessly, but I would prefer to be safe rather than
> > > sorry here and leave the explicit casts from int/long to double alone.
> > >
> > >
> >
> > Are there any tests that expose these errors?
> >
> >
> The ones we discovered, yes. IIRC, they were all of the form long/long or
> long*long embedded as below in a larger floating point computations.
See my other post. There are still some such (double) casts in the
code which Eclipse has not flagged.
AFAICT, Eclipse only flags as unnecessary the casts that would be done anyway.
> > Seems to me that if an apparently unnecessary cast is included in the
> > code, this should also be documented so it is clear why it is there.
> >
> >
> Or you could look at it as the cast itself as clarifying intent ;)
If only ...
> Phil
>
>
> >
> > > Phil
> > >
> > >
> > >
> > > > Modified:
> > > >
> > > >
> > >
> commons/proper/math/trunk/src/java/org/apache/commons/math/stat/regression/SimpleRegression.java
> > >
> > >
> > > > URL:
> > > >
> > > >
> > >
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/java/org/apache/commons/math/stat/regression/SimpleRegression.java?rev=762116&r1=762115&r2=762116&view=diff
> > >
> ==============================================================================
> > >
> > >
> > > > ---
> > > >
> > > >
> > >
> commons/proper/math/trunk/src/java/org/apache/commons/math/stat/regression/SimpleRegression.java
> > > (original)
> > >
> > >
> > > > +++
> > > >
> > > >
> > >
> commons/proper/math/trunk/src/java/org/apache/commons/math/stat/regression/SimpleRegression.java
> > > Sun Apr 5 16:48:53 2009
> > >
> > >
> > > > @@ -124,11 +124,11 @@
> > > > } else {
> > > > double dx = x - xbar;
> > > > double dy = y - ybar;
> > > > - sumXX += dx * dx * (double) n / (double) (n + 1.0);
> > > > - sumYY += dy * dy * (double) n / (double) (n + 1.0);
> > > > - sumXY += dx * dy * (double) n / (double) (n + 1.0);
> > > > - xbar += dx / (double) (n + 1.0);
> > > > - ybar += dy / (double) (n + 1.0);
> > > > + sumXX += dx * dx * n / (n + 1.0);
> > > > + sumYY += dy * dy * n / (n + 1.0);
> > > > + sumXY += dx * dy * n / (n + 1.0);
> > > > + xbar += dx / (n + 1.0);
> > > > + ybar += dy / (n + 1.0);
> > > > }
> > > > sumX += x;
> > > > sumY += y;
> > > > @@ -157,11 +157,11 @@
> > > > if (n > 0) {
> > > > double dx = x - xbar;
> > > > double dy = y - ybar;
> > > > - sumXX -= dx * dx * (double) n / (double) (n - 1.0);
> > > > - sumYY -= dy * dy * (double) n / (double) (n - 1.0);
> > > > - sumXY -= dx * dy * (double) n / (double) (n - 1.0);
> > > > - xbar -= dx / (double) (n - 1.0);
> > > > - ybar -= dy / (double) (n - 1.0);
> > > > + sumXX -= dx * dx * n / (n - 1.0);
> > > > + sumYY -= dy * dy * n / (n - 1.0);
> > > > + sumXY -= dx * dy * n / (n - 1.0);
> > > > + xbar -= dx / (n - 1.0);
> > > > + ybar -= dy / (n - 1.0);
> > > > sumX -= x;
> > > > sumY -= y;
> > > > n--;
> > > > @@ -410,7 +410,7 @@
> > > > if (n < 3) {
> > > > return Double.NaN;
> > > > }
> > > > - return getSumSquaredErrors() / (double) (n - 2);
> > > > + return getSumSquaredErrors() / (n - 2);
> > > > }
> > > > /**
> > > > @@ -468,7 +468,7 @@
> > > > */
> > > > public double getInterceptStdErr() {
> > > > return Math.sqrt(
> > > > - getMeanSquareError() * ((1d / (double) n) + (xbar * xbar)
> /
> > > >
> > > >
> > > sumXX));
> > >
> > >
> > > > + getMeanSquareError() * ((1d / n) + (xbar * xbar) /
> sumXX));
> > > > }
> > > > /**
> > > > @@ -589,7 +589,7 @@
> > > > * @return the intercept of the regression line
> > > > */
> > > > private double getIntercept(double slope) {
> > > > - return (sumY - slope * sumX) / ((double) n);
> > > > + return (sumY - slope * sumX) / (n);
> > > > }
> > > > /**
> > > >
> > > > Modified:
> > > >
> > > >
> > >
> commons/proper/math/trunk/src/java/org/apache/commons/math/util/MathUtils.java
> > >
> > >
> > > > URL:
> > > >
> > > >
> > >
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/java/org/apache/commons/math/util/MathUtils.java?rev=762116&r1=762115&r2=762116&view=diff
> > >
> ==============================================================================
> > >
> > >
> > > > ---
> > > >
> > > >
> > >
> commons/proper/math/trunk/src/java/org/apache/commons/math/util/MathUtils.java
> > > (original)
> > >
> > >
> > > > +++
> > > >
> > > >
> > >
> commons/proper/math/trunk/src/java/org/apache/commons/math/util/MathUtils.java
> > > Sun Apr 5 16:48:53 2009
> > >
> > >
> > > > @@ -307,7 +307,7 @@
> > > > return 0;
> > > > }
> > > > if ((k == 1) || (k == n - 1)) {
> > > > - return Math.log((double) n);
> > > > + return Math.log(n);
> > > > }
> > > > /*
> > > > @@ -337,12 +337,12 @@
> > > > // n!/(n-k)!
> > > > for (int i = n - k + 1; i <= n; i++) {
> > > > - logSum += Math.log((double) i);
> > > > + logSum += Math.log(i);
> > > > }
> > > > // divide by k!
> > > > for (int i = 2; i <= k; i++) {
> > > > - logSum -= Math.log((double) i);
> > > > + logSum -= Math.log(i);
> > > > }
> > > > return logSum; @@ -523,7 +523,7 @@
> > > > }
> > > > double logSum = 0;
> > > > for (int i = 2; i <= n; i++) {
> > > > - logSum += Math.log((double)i);
> > > > + logSum += Math.log(i);
> > > > }
> > > > return logSum;
> > > > }
> > > >
> > > > Modified:
> > > >
> > > >
> > >
> commons/proper/math/trunk/src/java/org/apache/commons/math/util/TransformerMap.java
> > >
> > >
> > > > URL:
> > > >
> > > >
> > >
> http://svn.apache.org/viewvc/commons/proper/math/trunk/src/java/org/apache/commons/math/util/TransformerMap.java?rev=762116&r1=762115&r2=762116&view=diff
> > >
> ==============================================================================
> > >
> > >
> > > > ---
> > > >
> > > >
> > >
> commons/proper/math/trunk/src/java/org/apache/commons/math/util/TransformerMap.java
> > > (original)
> > >
> > >
> > > > +++
> > > >
> > > >
> > >
> commons/proper/math/trunk/src/java/org/apache/commons/math/util/TransformerMap.java
> > > Sun Apr 5 16:48:53 2009
> > >
> > >
> > > > @@ -79,7 +79,7 @@
> > > > * @return the mapped NumberTransformer or null.
> > > > */
> > > > public NumberTransformer getTransformer(Class<?> key) {
> > > > - return (NumberTransformer) map.get(key);
> > > > + return map.get(key);
> > > > }
> > > > /**
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> ---------------------------------------------------------------------
> > > 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
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org