I’ve got a long-running project that’s very nearly completed, in which I’m converting some old IDL code to Java for the modeling of Martian photometry as observed by HiRISE. I use JFreeChart in this project (and all of my projects that require charts, in fact).

I happened to be digging into the source code for the NormalDistributionFunction2D class and I found this piece of madness:

public double getValue(double x) {
return Math.exp(-1.0 * (x - this.mean) * (x - this.mean)
/ (2 * this.std * this.std)) / Math.sqrt(2 * Math.PI
* this.std * this.std);
}

“std” is the function’s standard deviation, set at construction.

The madness is in the denominator of the function: the square root of…a constant times the standard deviation squared. Isn’t that just the standard deviation times the square root of a constant? And the square root of a constant is itself a constant.

So why isn’t the denominator just a constant times the standard deviation? Why have four multiplications in there followed by a call to the math library? This expression is evaluated every single time the NormalDistributionFunction2D’s getValue method is called!

Because the standard deviation could be negative! Wait, what? No, it can’t. Even if there was some sense to it, the only computation in this code involves squaring the standard deviation, effectively stripping it of its sign. The denominator really ought to be a pre-computed constant times the standard deviation.

Also, since the class doesn’t let standard deviation be changed after construction, *the entire denominator can be pre-computed at construction*, as can the denominator of the exponent used in the numerator. Neither component ever changes once the object has been constructed.

Sigh.

This is in version 1.0.10 of JFreeChart, by the way. Maybe it’s been fixed by now. I probably ought to be a good open source citizen and submit a fix myself if it’s not been fixed.

### Like this:

Like Loading...

*Related*

February 6, 2009 at 10:57 am

I opened a JFreeChart bug report (#2572016 ) and will look at this as soon as I can. I don’t remember the source I used for the formula, but I could well have got it wrong.

February 6, 2009 at 3:04 pm

Oh, geez. Thanks, Dave. The formula certainly is mathematically right, and not pre-computing the constants probably doesn’t affect all (most? many?) users. It might affect my own use, which is to predict how the HiRISE camera aboard the Mars Reconnaissance Orbiter responds to Mars, and in which there will be quite a few calls to getValue once I revise my own code to use NormalDistributionFunction2D.

We use JFreeChart all over the place, by the way: I use it for plotting the camera’s predicted thermal profile, for plotting the aforementioned photometry, for plotting lookup table effects in 14-bit to 8-bit data conversion, for plotting stereo pair geometry, and for plotting some basic details of the camera (CCD light response curves, temperature sensor conversion, etc.).