Opened 4 years ago

Last modified 2 years ago

#19461 needs_work enhancement

Pochhammer symbols

Reported by: rws Owned by:
Priority: major Milestone: sage-7.4
Component: symbolics Keywords: falling_factorial, rising_factorial
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description (last modified by rws)

While Sage already has falling_factorial and rising_factorial functions they are not symbolic. Always using gamma or product expansions may be inconvenient in some cases, and does not offer the option to rewrite expressions in terms of them. Especially the product form cannot be easily rewritten as gamma expression. So the product expansion should be made optional.

Change History (9)

comment:1 follow-up: Changed 3 years ago by arminstraub

  • Milestone changed from sage-6.10 to sage-duplicate/invalid/wontfix
  • Status changed from new to needs_review

It seems that this has been fixed in recent versions of Sage. At least, the functions can now be used as part of symbolic expressions:

sage: rising_factorial(x, 3)
(x + 2)*(x + 1)*x
sage: rising_factorial(x, x)
gamma(2*x)/gamma(x)

I have therefore set the ticket to "invalid". Please change back if I am misreading the description.

comment:2 Changed 3 years ago by arminstraub

  • Reviewers set to Armin Straub
  • Status changed from needs_review to positive_review

On another ticket I was told to also set these to positive review... If that's not universally true, please let me know.

comment:3 in reply to: ↑ 1 ; follow-up: Changed 3 years ago by rws

  • Milestone changed from sage-duplicate/invalid/wontfix to sage-7.4
  • Reviewers Armin Straub deleted
  • Status changed from positive_review to needs_work

Replying to arminstraub:

It seems that this has been fixed in recent versions of Sage. At least, the functions can now be used as part of symbolic expressions:

sage: rising_factorial(x, 3)
(x + 2)*(x + 1)*x
sage: rising_factorial(x, x)
gamma(2*x)/gamma(x)

It was not fixed. I would like at least the option of not converting to gamma fractions, which is impossible without a symbolic function. Also, you don't want rising_factorial(x, 3000) expanded. The function itself is worthwhile to have.

On another ticket I was told to also set these to positive review.

I think this applies only to tickets that you have started.

Last edited 3 years ago by rws (previous) (diff)

comment:4 in reply to: ↑ 3 ; follow-up: Changed 3 years ago by arminstraub

Replying to rws:

Also, you don't want rising_factorial(x, 3000) expanded.

I would disagree on that part. If I ever do explicitly write rising_factorial(x, 3000), then I would like it expanded (just as I appreciate that factorial(3000) returns a huge integer). For what it's worth, that is what Mathematica does.

Also note that we can express the Pochhammers using binomial coefficents. And, binomial(x,3000) (which equals falling_factorial(x,3000)/factorial(3000)) is returned in expanded form (and I am glad it is). The worst alternative would be a change in behaviour at a random value, which is deemed "too large" for expansion.

The function itself is worthwhile to have.

That's of course a different matter. Maybe you could update the description? (The part after "i.e." and the second sentence about implementations like Gosper do not apply anymore.)

On another ticket I was told to also set these to positive review.

I think this applies only to tickets that you have started.

I see, thanks!

comment:5 Changed 3 years ago by rws

  • Description modified (diff)
  • Priority changed from major to minor

comment:6 Changed 3 years ago by pelegm

  • Keywords falling_factorial rising_factorial added

comment:7 in reply to: ↑ 4 Changed 3 years ago by rws

Replying to arminstraub:

Maybe you could update the description? (The part after "i.e." and the second sentence about implementations like Gosper do not apply anymore.)

Actually it does apply. Gosper's and similar algorithms need to transform expressions to gamma expressions, and that's very simple with a rising_factorial(x,5). The information is lost with (x + 4)*(x + 3)*(x + 2)*(x + 1)*x.

comment:8 Changed 3 years ago by rws

  • Description modified (diff)
  • Priority changed from minor to major

comment:9 Changed 2 years ago by rws

The planned behaviour would be expanding as the default. Adding hold=True will prevent it.

However, it seems at first the ticket cannot be implemented unless matrices etc. coerce into SR:

File "src/sage/arith/misc.py", line 4377, in sage.arith.misc.falling_factorial
Failed example:
    falling_factorial(A, 2) # A(A - I)
...
      File "sage/symbolic/function.pyx", line 996, in sage.symbolic.function.BuiltinFunction.__call__ (build/cythonized/sage/symbolic/function.cpp:11436)
        res = super(BuiltinFunction, self).__call__(
      File "sage/symbolic/function.pyx", line 474, in sage.symbolic.function.Function.__call__ (build/cythonized/sage/symbolic/function.cpp:6321)
        raise TypeError("cannot coerce arguments: %s" % (err))
    TypeError: cannot coerce arguments: no canonical coercion from Full MatrixSpace of 4 by 4 dense matrices over Integer Ring to Symbolic Ring

The reason is that making the symbolic rising/falling_factorial the default will switch on some checks that are in symbolic/function.pyx so the doctests in arith/misc.py will fail unless the arith/ version is explicitly called, which is a viable option IMHO. Compare #17489 which is stuck because the overriding of the arith/ version (of factorial()) causes failure of the algorithm keyword.

The pragmatic solution would be to accept there are two versions of the functions rising/falling_/factorial() (more?) and that the versions in arith/ must be explicitly called if the symbolic ones don't suffice.

Last edited 2 years ago by rws (previous) (diff)
Note: See TracTickets for help on using tickets.