commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles <gil...@harfang.homelinux.org>
Subject Re: [NUMBERS] Proposal for refactoring and extension of Gamma functions.
Date Wed, 07 Jun 2017 13:52:36 GMT
Hello.

On Wed, 07 Jun 2017 12:53:42 +0000, Amey Jadiye wrote:
> Hi Gilles,
>
> Thanks for inputs, I will generate some stats with jmh let's see if 
> we are
> getting benefits with my proposal.
>
> Mean time I have submitted test cases[1] for NUMBERS-38, please 
> review.

Tabs is a "no-no". ;-)

I find it quite unusual to test equality of numbers by first
converting them to (truncated) strings.

Finally, I think that it is not that helpful to have the _same_
code as the one being checked, duplicated in the unit test class.
[I mean, it is obvious that they will give the same answer...]
It is more robust to run a code externally, and copy/paste the
results as an array of "reference values" to be checked against.
[See e.g. in class "GammaTest".]

But don't rush into making another pass at it before we decide
whether it is necessary at all to have more unit tests: If the
code is somehow covered adequately by the other tests, we might
just resolve the issue as "Not a problem" (and a comment in the
test class).

IMO, it would be more useful to figure out whether this bit of
code should be renamed: it would be wrong to have a class called
"LanczosApproximation" in the public API if what it does is not
what would one expect from such a class.
And consequently, also, we need (before the first release) to
figure out whether it is possible to make it "internal", and fix
"GammaDistribution" accordingly.


Best,
Gilles

>
> [1] https://github.com/apache/commons-numbers/pull/5
>
> Regards,
> Amey
>
> On Wed, Jun 7, 2017, 4:44 AM Gilles <gilles@harfang.homelinux.org> 
> wrote:
>
>> Hi Amey.
>>
>> On Wed, 7 Jun 2017 01:15:01 +0530, Amey Jadiye wrote:
>> > Hi,
>> >
>> > Gamma class is written keeping in mind that it should handle fair
>> > situation
>> > (if n < 20) it computes with normal gamma function else it uses
>> > LanczosApproximation
>> > for higher numbers, for now I think we should keep it behaviour as 
>> it
>> > is
>> > and do just code segrigation, by segregation of there 
>> functionalities
>> > we
>> > are making it switchable so Gamma class by optional providing
>> > constructor
>> > args of Lanzoz, Stinling or Spouge's algo, same thing we have to 
>> do
>> > in
>> > Math's Gamma distribution.
>> >
>> > Ex.
>> > Gamma g = Gamma(Algo.LANCZOS));
>>
>> Just from the look of it, it is difficult to figure out whether
>> alternative algorithms are useful when numbers are represented
>> as 64-bits "double".
>>
>> If you are willing to go that route, you should provide code that
>> shows the advantages (speed and/or precision).
>>
>> Now, if we want to support an arbirary precision number type[1],
>> then the alternate algorithms that can provide accuracy below
>> ~1e-16 would certainly be worth considering.[2]
>>
>> > I'd like other people from group chime in discussion.
>>
>>
>> Regards,
>> Gilles
>>
>> [1] See the "Dfp" class in CM's "o.a.c.math4.dfp" package.
>> [2] But IMO this should be postponed to after 1.0 since there
>>      is perhaps a need of a general discussion about designing
>>      high-precision algorithms (and whether there are volunteers
>>      to develop and support them in the long term).
>>      I may be wrong, but somehow I have the intuition that people
>>      requiring those functionalities might not be using Java...
>>
>> >
>> > Regards,
>> > Amey
>> >
>> > On Tue, Jun 6, 2017 at 3:31 AM, Gilles 
>> <gilles@harfang.homelinux.org>
>> > wrote:
>> >
>> >> On Tue, 6 Jun 2017 01:14:38 +0530, Amey Jadiye wrote:
>> >>
>> >>> Hi All,
>> >>>
>> >>> Coming from discussion happened here
>> >>> https://issues.apache.org/jira/browse/NUMBERS-38
>> >>>
>> >>> As Gamma is nothing but advanced factorial function 
>> gamma(n)=(n-1)!
>> >>> with
>> >>> advantages like we can have factorial of whole numbers as well 
>> as
>> >>> factional. Now as [Gamma functions (
>> >>> https://en.wikipedia.org/wiki/Gamma_function ) which is having
>> >>> general
>> >>> formula {{Gamma( x ) = integral( t^(x-1) e^(-t), t = 0 ..
>> >>> infinity)}} is a
>> >>> plane old base function however Lanczos approximation / 
>> Stirling's
>> >>> approximation /Spouge's Approximation  *is a* gamma function so
>> >>> they
>> >>> should
>> >>> be extend Gamma.
>> >>>
>> >>> Exact algorithm and formulas here :
>> >>>  - Lanczo's Approximation -
>> >>> https://en.wikipedia.org/wiki/Lanczos_approximation
>> >>>  - Stirling's Approximation  -
>> >>> https://en.wikipedia.org/wiki/Stirling%27s_approximation
>> >>>  - Spouge's Approximation -
>> >>> https://en.wikipedia.org/wiki/Spouge%27s_approximation
>> >>>
>> >>> Why to refactor code is because basic gamma function computes 
>> not
>> >>> so
>> >>> accurate/precision values so someone who need quick computation
>> >>> without
>> >>> precision can choose it, while someone who need precision overs
>> >>> cost of
>> >>> performance (Lanczos approximation is accurate so its slow takes
>> >>> more cpu
>> >>> cycle) can choose which algorithm they want.
>> >>>
>> >>> for some scientific application all values should be computed 
>> with
>> >>> great
>> >>> precision, with out Gamma class no choice is given for choosing
>> >>> which
>> >>> algorithm user want.
>> >>>
>> >>> I'm proposing to create something like:
>> >>>
>> >>> Gamma gammaFun = new Gamma(); gammaFun.value( x );
>> >>> Gamma gammaFun = new LanczosGamma(); gammaFun.value( x );
>> >>> Gamma gammaFun = new StirlingsGamma(); gammaFun.value( x );
>> >>> Gamma gammaFun = new SpougesGamma(); gammaFun.value( x );
>> >>>
>> >>> Also as the class name suggestion {{LanczosApproximation}} it
>> >>> should
>> >>> execute/implement full *Lanczos Algoritham* but we are just
>> >>> computing
>> >>> coefficients in that class which is incorrect, so just 
>> refactoring
>> >>> is
>> >>> needed which wont break any dependency of this class anywhere.
>> >>> (will
>> >>> modify
>> >>> code such way).
>> >>>
>> >>> let me know your thoughts?
>> >>>
>> >>>
>> >> I've added a comment (about the multiple implementations) on the
>> >> JIRA page.
>> >>
>> >> I agree that if the class currently named "LanczosApproximation" 
>> is
>> >> not what the references define as "Lanczos' approximation", it
>> >> should
>> >> be renamed.
>> >> My preference would be to "hide" it inside the "Gamma" class, if 
>> we
>> >> can sort out how to modify the "GammaDistribution" class (in 
>> Commons
>> >> Math) accordingly.
>> >>
>> >> Regards,
>> >> Gilles
>> >>
>> >>


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


Mime
View raw message