Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pochhammer symbols #19461

Open
rwst opened this issue Oct 23, 2015 · 11 comments
Open

Pochhammer symbols #19461

rwst opened this issue Oct 23, 2015 · 11 comments

Comments

@rwst
Copy link

rwst commented Oct 23, 2015

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.

Component: symbolics

Keywords: falling_factorial, rising_factorial

Issue created by migration from https://trac.sagemath.org/ticket/19461

@rwst rwst added this to the sage-6.10 milestone Oct 23, 2015
@arminstraub
Copy link

comment:1

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.

@arminstraub arminstraub removed this from the sage-6.10 milestone Aug 18, 2016
@arminstraub
Copy link

comment:2

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

@arminstraub
Copy link

Reviewer: Armin Straub

@rwst
Copy link
Author

rwst commented Aug 19, 2016

comment:3

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.

@rwst
Copy link
Author

rwst commented Aug 19, 2016

Changed reviewer from Armin Straub to none

@rwst rwst added this to the sage-7.4 milestone Aug 19, 2016
@arminstraub
Copy link

comment:4

Replying to @rwst:

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!

@rwst

This comment has been minimized.

@pelegm
Copy link
Contributor

pelegm commented Dec 7, 2016

Changed keywords from none to falling_factorial, rising_factorial

@rwst
Copy link
Author

rwst commented Dec 8, 2016

comment:7

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.

@rwst

This comment has been minimized.

@rwst
Copy link
Author

rwst commented Dec 29, 2016

comment:9

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants