Opened 3 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 )
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: ↓ 3 Changed 3 years ago by
- Milestone changed from sage-6.10 to sage-duplicate/invalid/wontfix
- Status changed from new to needs_review
comment:2 Changed 3 years ago by
- 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: ↓ 4 Changed 3 years ago by
- 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.
comment:4 in reply to: ↑ 3 ; follow-up: ↓ 7 Changed 3 years ago by
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
- Description modified (diff)
- Priority changed from major to minor
comment:6 Changed 2 years ago by
- Keywords falling_factorial rising_factorial added
comment:7 in reply to: ↑ 4 Changed 2 years ago by
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 2 years ago by
- Description modified (diff)
- Priority changed from minor to major
comment:9 Changed 2 years ago by
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.
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:
I have therefore set the ticket to "invalid". Please change back if I am misreading the description.