[
https://issues.apache.org/jira/browse/NUMBERS-178?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17453159#comment-17453159
]
Gilles Sadowski commented on NUMBERS-178:
-----------------------------------------
{quote}
{code}
FactorialDouble f = FactorialDouble.create();
{code}
{quote}
Such usage now seems mildly confusing, as nothing is "created".
I suggest deprecating all the methods; usage becoming
{code}
int n = 123;
double nFact = FactorialDouble.of(n);
{code}
> FactorialDouble can tabulate the representable factorials
> ---------------------------------------------------------
>
> Key: NUMBERS-178
> URL: https://issues.apache.org/jira/browse/NUMBERS-178
> Project: Commons Numbers
> Issue Type: Improvement
> Components: combinatorics
> Affects Versions: 1.0
> Reporter: Alex Herbert
> Priority: Trivial
> Fix For: 1.1
>
>
> The updated Gamma function (see NUMBERS-174) tabulates all representable
> factorials.
> This suggests one of the following changes:
> # The FactorialDouble class can call the Gamma function to obtain the values.
> # The FactorialDouble class can also tabulate the values and not use a
> dependency on the Gamma class
> Note that if the call is made to the Gamma class the method is effectively:
> {code:java}
> public static double value(int n) {
> // The Gamma class has all factorial values tabulated.
> return tgamma(n + 1);
> }
> static double tgamma(double z) {
> // Handle integers
> if (Math.rint(z) == z) {
> if (z <= 0) {
> // Pole error
> return Double.NaN;
> }
> if (z <= MAX_GAMMA_Z) {
> // Gamma(n) = (n-1)!
> return FACTORIAL[(int) z - 1];
> }
> // Overflow
> return Double.POSITIVE_INFINITY;
> }
> // ... Not used
> }
> {code}
> So calling the Gamma method has a round trip of the integer to a double, a
> check it is an integer, then a conversion back to an integer.
> The FactorialDouble class also has a cache of the values. So all the values
> are precomputed once and stored for the instance.
> I suggest updating the class to remove the cache and just storing the 171
> representable double values for values up to 170!. The public API can remain
> the same but the cache methods marked as deprecated. The value method then
> becomes:
> {code:java}
> public double value(int n) {
> if (n < 0) {
> throw new CombinatoricsException(CombinatoricsException.NEGATIVE,
> n);
> }
> if (n < FACTORIAL.length) {
> // Cache of precomputed values up to 170!
> return FACTORIAL[n];
> }
> return Double.POSITIVE_INFINITY;
> }
> {code}
> Note:
> This class is a similar implementation to the LogFactorial class. In that
> case the maximum representable LogFactorial is very big and the cache
> functionality makes sense. For a maximum cache size of 171 this caching
> functionality seems unnecessary.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
