sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.43k stars 479 forks source link

Fix product(-x, x, 1, SR.var('n')) to return (-1)^n*factorial(n) #31557

Open slel opened 3 years ago

slel commented 3 years ago

From #30520.

Observed:

sage: product(-x, x, 1, SR.var('n'))
#0: simplify_product(product='product(-_SAGE_VAR_x,_SAGE_VAR_x,1,_SAGE_VAR_n))
...
Traceback (most recent call last)
...
RuntimeError: ECL says: Error executing code in Maxima:
factorial: factorial of negative integer -1 not defined.

or equivalently:

sage: from sage.calculus.calculus import symbolic_product
sage: x, n = SR.var('x, n')
sage: symbolic_product(-x, x, 1, n)
#0: simplify_product(product='product(-_SAGE_VAR_x,_SAGE_VAR_x,1,_SAGE_VAR_n))
...
Traceback (most recent call last)
...
RuntimeError: ECL says: Error executing code in Maxima:
factorial: factorial of negative integer -1 not defined.

Expected:

sage: from sage.calculus.calculus import symbolic_product
sage: x, n = SR.var('x, n')
sage: symbolic_product(-x, x, 1, n)
(-1)^n*factorial(n)

Upstream: Reported upstream. Developers acknowledge bug.

CC: @robert-dodier @slel

Component: symbolics

Keywords: symbolic_product, maxima

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

slel commented 3 years ago
comment:1

Robert Dodier's analysis at #30520, comment 16 points to:

https://github.com/andrejv/maxima/blob/5.36.0.1/share/solve_rec/solve_rec.mac#L1095-L1112

robert-dodier commented 3 years ago
comment:2

I've pushed commit c584c24 on maxima-code/master to fix this bug.

slel commented 3 years ago

Upstream: Fixed upstream, but not in a stable release.

slel commented 3 years ago

Description changed:

--- 
+++ 
@@ -11,7 +11,7 @@
 RuntimeError: ECL says: Error executing code in Maxima:
 factorial: factorial of negative integer -1 not defined.

-which amounts to: +or equivalently:

 sage: from sage.calculus.calculus import symbolic_product
slel commented 3 years ago
comment:3

Thanks for fixing this and for the heads-up.

This should be applied as a patch in Sage until a Maxima release has this and we upgrade to it.

slel commented 3 years ago

Description changed:

--- 
+++ 
@@ -34,3 +34,6 @@
 (-1)^n*factorial(n)

+Fixed upstream by this commit: + +- https://sourceforge.net/p/maxima/code/ci/c584c24ac0542a4ada435aad41f1e5adf892bbc2/

slel commented 3 years ago
comment:4

Adding link to commit to ticket description.

robert-dodier commented 3 years ago
comment:5

Oops, spoke too soon. Commit c584c24 causes a stack overflow in some other code (namely simplify_sum). Commit c584c24 has been reverted, and I'm working on an update. Sorry to say the problem isn't actually fixed yet.

mkoeppe commented 3 years ago
comment:6

Moving to 9.4, as 9.3 has been released.

DaveWitteMorris commented 3 years ago

Description changed:

--- 
+++ 
@@ -34,6 +34,3 @@
 (-1)^n*factorial(n)

-Fixed upstream by this commit:

-- https://sourceforge.net/p/maxima/code/ci/c584c24ac0542a4ada435aad41f1e5adf892bbc2/

DaveWitteMorris commented 3 years ago

Changed upstream from Fixed upstream, but not in a stable release. to Reported upstream. Developers acknowledge bug.