-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Description
In pursuit of a better gamma function to improve #3532, in addition to the Bernoulli numbers #3551, we will need rising factorials
So some questions:
-
Are you comfortable extending
factorial(n)with a new signaturefactorial(x, n)where basicallyxis any numerical value andnis any kind of integer, and whennis positive you get the rising factorial and when it is negative you get the falling factorial? Or do you prefer to have new functions - either two of themrisingFactorialandfallingFactorial, or maybe squash them into one name likeshiftingFactorialthat works with positive or negativen? [For the record, I slightly prefer to have just the one name,factorial, that can be used for all three.] -
Once rising factorials are in place, a very convenient way in terms of code simplicity to implement
factorial(n)is just as the rising factorial$1^{(n)}$ . The naive implementation of rising factorials is iterated multiplication. The first pass better than this is "binary splitting" where you pick h about half of n when n is over some small threshold and compute$x^{(n)} = x^{(h)}\cdot(x+h)^{(n-h)}$ . That's actually already a significant improvement over the naive, because you tend to do many more "easy" multiplications and fewer "hard" ones. However, we currently have this special trick for factorial of BigNumber that cuts the total number of multiplications in half by exploiting the fact that triangular numbers are proportional to$n(n+1)$ . (There are even more sophisticated methods but I think they are overkill for mathjs.)
However, if we switched to this convenient definition of factorial in terms of rising factorial, we would end up with the following benchmarks:
bigFactorial for 8 1.26 µs ±5.95%
new bigFactorial for 8 0.80 µs ±1.18%
split factorial for 8 1.10 µs ±8.00%
bigFactorial for 20 3.06 µs ±3.25%
new bigFactorial for 20 2.10 µs ±23.63%
split factorial for 20 2.83 µs ±4.25%
bigFactorial for 600 220.03 µs ±0.85%
new bigFactorial for 600 110.35 µs ±0.66%
split factorial for 600 126.93 µs ±0.73%
bigFactorial for 1.5K 545.71 µs ±0.53%
new bigFactorial for 1.5K 273.70 µs ±0.63%
split factorial for 1.5K 312.90 µs ±1.59%
bigFactorial for 10K 3633.01 µs ±0.38%
new bigFactorial for 10K 2454.87 µs ±0.49%
split factorial for 10K 2223.74 µs ±0.63%
bigFactorial for 30K 11066.99 µs ±0.41%
new bigFactorial for 30K 8634.30 µs ±2.67%
split factorial for 30K 8307.91 µs ±0.23%
So you can see that the split factorial (i.e. based on the binary-splitting rising factorial) actually outperforms the current clever trick for large factorials, even though it's doing twice as many multiplies -- it's just mostly doing much easier ones. (I didn't search for the breakeven point, maybe it's a something like 5000!.) But for intermediate factorials in the 600! to 1500! range, the split factorial is about 15% slower. Would that sort of slowdown be acceptable for the sake of code simplicity, or do you want code which checks the number of terms and if it's in that middle range it uses the current trick and at some threshold it switches to the split factorial? [My personal take is that Javascript is never going to actually be a high-performance engine, so it's not worth convoluting our code for modest speedup in intermediate cases, but I am open to counterpoint.]
Note that in this refactor, if we go ahead and base factorial on the rising factorial, then it will make sense to move the factorial code into the factorial function, rather than in the gamma function where it is now. I actually think that's an organizational improvement, since at the moment if you call factorial on a bignumber, it delegates to gamma, which right now only works for integers and internally just calls a function bigFactorial, which seems kind of needlessly roundabout. The eventual idea is that gamma will work for BigNumbers in general, not just integers, and depend on the rising factorials, whatever they are called, and so there won't be any point in the factorial implementation being in the gamma.js source file, whatever it is.
I will hold off on implementing rising factorials and the more complete gamma function until I get some feedback on the rising factorial syntax and factorial implementation questions above. Thanks.