commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles Sadowski <gil...@harfang.homelinux.org>
Subject Re: [Math] "Normal" GaussianFitter
Date Tue, 15 Feb 2011 23:55:39 GMT
Hi.

> >However, as I've raised in
> >   https://issues.apache.org/jira/browse/MATH-512
> >I think that "GaussianFitter" needs some refactoring.
> >Maybe you could try to see what changes would be necessary to deal with
> >MATH-512 and such that the upgraded "GaussianFitter" will meet with your
> >requirements at the same time.
> 
> I went ahead and implemented a "Normal" GaussianFitter.  Turns out there's not that much
difference in the resulting best fit parameters, and it seems four parameters are better than
three.

Maybe the data you are fitting are not from a "Normal" Gaussian...
[You add one more parameter and get a better fit but I think that it does
not necessarily mean that your data is indeed best represented by the sum
of a constant and a Gaussian.]

> I agree with your comments in 512.  I can refactor the classes as suggested, just let
me know...

Yes, please give it a try; however before you do it, I'm thinking of another
improvement: Why not move the defintion of "ParametricGaussianFunction" to
the "Gaussian" class (in package "function")? [I've done just that in the
attached copy of "Gaussian.java".]
Thinking about it, it seems that the "ParametricRealFunction" interface
might be of a more general use than just in fitting, so I'd move it over to
the package "analysis" (where other function interfaces are defined).

> Also if commons-math is interested I can submit the classes for the "Normal" GaussianFitter.
 I thought about combining the two and I think the implementation will be much cleaner if
there are two separate implementations.

At first sight, I don't think so. Once refactored, the "GaussianFitter"
could have 2 constructors, one would take a "Gaussian.Parametric" function
(that would be fitting the "Normal" case) and the other would take a
"FourParameterGaussianParametricFunction".
But I still wonder if the use of the word "Gaussian" in the latter is really
appropriate. I'd even say that such a function shouldn't be in CM at all,
unless one is willing to accept the implementation of a
"FiveParameterGaussianParametricFunction" (where we would add, say, a
quadratic function) and a "SixParameterGaussian...", etc.

> Maybe the one that reflects the Wikipedia definition should be called "GaussianFitter"
and the current one should be Called "GaussianFitterWithHeightFactor"...
> 
> or perhaps
> 
> ThreeParameterGaussianFitter
> FourParameterGaussianFitter...

As explained above, I'd rather leave such fuzzy names to the user-code
layer.


Regards,
Gilles

Mime
View raw message