[
https://issues.apache.org/jira/browse/NUMBERS-178?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alex Herbert resolved NUMBERS-178.
----------------------------------
Resolution: Implemented
Updated in commit:
274818ccab651e51a47985a1ea54ef069b524adb
The {{withCache(int cacheSize)}} method now returns a reference to the single
instance of FactorialDouble.
I did not see the point in validating the cacheSize is positive so the
implementation is:
{code:java}
/**
* @deprecated Since 1.1 the cache size argument is ignored. Use {@link
#create()} to obtain an instance.
*
* <p>As of Numbers 1.1 all representable factorials up to {@code n=170} are
tabulated.
* This method returns a reference to the same object.
*
* @param cacheSize Ignored.
* @return instance
*/
@Deprecated
public FactorialDouble withCache(final int cacheSize) {
return this;
}
{code}
This introduces a behavioural change where this code was invalid in Numbers 1.0
but is now possible in Numbers 1.1:
{code:java}
FactorialDouble f = FactorialDouble.create().withCache(-100);
{code}
The method is deprecated and should be replaced with a simple call to create():
{code:java}
FactorialDouble f = FactorialDouble.create();
{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)
