Opened 8 years ago
Closed 6 years ago
#17505 closed enhancement (fixed)
implement symbolic product
Reported by:  rws  Owned by:  

Priority:  major  Milestone:  sageduplicate/invalid/wontfix 
Component:  symbolics  Keywords:  
Cc:  charpent  Merged in:  
Authors:  Ralf Stephan  Reviewers:  Emmanuel Charpentier 
Report Upstream:  N/A  Work issues:  
Branch:  u/rws/implement_symbolic_product  Commit:  5779423aac340d1d7dc415da287b9af5e0e1f381 
Dependencies:  Stopgaps: 
Description (last modified by )
The symbolic product is currently broken in Sage :
 It cannot be created in Sage :
sage: var("j,p", domain="integer") sage: X,Y=function("X,Y") sage: prod(X(j),j,1,p)  TypeError Traceback (most recent call last) <ipythoninput585e69544cbe9> in <module>() > 1 prod(X(j),j,Integer(1),p) /usr/local/sage8/src/sage/misc/misc_c.pyx in sage.misc.misc_c.prod (/usr/local/sage8/src/build/cythonized/sage/misc/misc_c.c:1596)() 69 70 > 71 def prod(x, z=None, Py_ssize_t recursion_cutoff=5): 72 """ 73 Return the product of the elements in the list x. TypeError: prod() takes at most 3 positional arguments (4 given) sage: product(X(j),j,1,p)  NameError Traceback (most recent call last) <ipythoninput64d04d74c7489> in <module>() > 1 product(X(j),j,Integer(1),p) NameError: name 'product' is not defined
At the moment anonymous functions named product
can be created via the Maxima pexpect
interface and they even behave as products in specific cases:
sage: maxima("prod(X(j),j,1,p)").sage().log().log_expand() sum(log(X(j)), j, 1, p)
The present ticket aims at creating a Sage function/method either evaluating the sum, or correctly creating a unevaluted symbolic product object.
For evaluation the ticket would have to decide which of (Maxima,SymPy?) would be used as default for this.
sage: import sympy sage: x = sympy.Symbol('x') sage: n = sympy.Symbol('n') sage: sympy.product(x, (x, 1, n)) factorial(n) sage: sympy.product(sin(x), (x, 1, n)) Product(sin(x), (x, 1, n))
Creating products by casting a Maxima expression via the library interface gives nonsense, see #17502.
Change History (40)
comment:1 Changed 8 years ago by
Description:  modified (diff) 

comment:2 Changed 8 years ago by
Milestone:  sage6.5 → sagewishlist 

comment:3 Changed 6 years ago by
comment:4 Changed 6 years ago by
Cc:  charpent added 

comment:5 followup: 6 Changed 6 years ago by
Description:  modified (diff) 

comment:6 Changed 6 years ago by
comment:7 Changed 6 years ago by
Description:  modified (diff) 

Milestone:  sagewishlist → sage8.0 
comment:8 Changed 6 years ago by
Branch:  → u/rws/implement_symbolic_product 

comment:9 Changed 6 years ago by
Commit:  → 647ff396cc17c8acb59043dad905ab882834017b 

do you mind adding giac='product'
?
since:
sage: giac('product(x, x, 1, n)') n! sage: _.sage() factorial(n)
New commits:
647ff39  17505: unevaluated symbolic product

comment:10 Changed 6 years ago by
Commit:  647ff396cc17c8acb59043dad905ab882834017b → e4769b5524ae5629db4189d06684646b22781efb 

Branch pushed to git repo; I updated commit sha1. New commits:
e4769b5  17505: symbolic product

comment:11 followup: 12 Changed 6 years ago by
Authors:  → Ralf Stephan 

Status:  new → needs_review 
This doesn't have the prod
interface (other ticket) and some docs are missing but everything should work. Question: where does SymPy differ from Maxima?
comment:12 followup: 17 Changed 6 years ago by
Status:  needs_review → needs_work 

Replying to rws:
This doesn't have the
prod
interface (other ticket) and some docs are missing but everything should work. Question: where does SymPy differ from Maxima?
First of all, thank you very much for this addition, which should enhance Sage's usefulness fo highschool/undergrad levels.
However, ptestlong
gives three failures :
 sage t long src/sage/calculus/calculus.py # 5 doctests failed sage t long src/sage/combinat/posets/posets.py # 1 doctest failed sage t long src/sage/homology/simplicial_complex.py # 1 doctest failed 
The second and third ones have been reported for 8.0.beta4 and are seen again in 8.0beta5. Nothing new here, so probably not related.
The third one is new :
charpent@asus16ec:/usr/local/sage8$ sage t long src/sage/calculus/calculus.py too many failed tests, not using stored timings Running doctests with ID 2017050814443993705f01. Git branch: t/17505/implement_symbolic_product Using optional=database_gap,giacpy_sage,git_trac,mpir,python2,sage Doctesting 1 file. sage t long src/sage/calculus/calculus.py ********************************************************************** File "src/sage/calculus/calculus.py", line 843, in sage.calculus.calculus.symbolic_prod Failed example: symbolic_prod(x + i*(i+1)/2, i, 1, 4) Exception raised: Traceback (most recent call last): File "/usr/local/sage8/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 509, in _run self.compile_and_execute(example, compiler, test.globs) File "/usr/local/sage8/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 872, in compile_and_execute exec(compiled, globs) File "<doctest sage.calculus.calculus.symbolic_prod[3]>", line 1, in <module> symbolic_prod(x + i*(i+Integer(1))/Integer(2), i, Integer(1), Integer(4)) File "/usr/local/sage8/local/lib/python2.7/sitepackages/sage/calculus/calculus.py", line 860, in symbolic_prod raise TypeError("need a multiplication variable") TypeError: need a multiplication variable ********************************************************************** File "src/sage/calculus/calculus.py", line 845, in sage.calculus.calculus.symbolic_prod Failed example: symbolic_prod(i^2, i, 1, 7) Exception raised: Traceback (most recent call last): File "/usr/local/sage8/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 509, in _run self.compile_and_execute(example, compiler, test.globs) File "/usr/local/sage8/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 872, in compile_and_execute exec(compiled, globs) File "<doctest sage.calculus.calculus.symbolic_prod[4]>", line 1, in <module> symbolic_prod(i**Integer(2), i, Integer(1), Integer(7)) File "/usr/local/sage8/local/lib/python2.7/sitepackages/sage/calculus/calculus.py", line 860, in symbolic_prod raise TypeError("need a multiplication variable") TypeError: need a multiplication variable ********************************************************************** File "src/sage/calculus/calculus.py", line 848, in sage.calculus.calculus.symbolic_prod Failed example: symbolic_prod(f(i), i, 1, 7) Exception raised: Traceback (most recent call last): File "/usr/local/sage8/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 509, in _run self.compile_and_execute(example, compiler, test.globs) File "/usr/local/sage8/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 872, in compile_and_execute exec(compiled, globs) File "<doctest sage.calculus.calculus.symbolic_prod[6]>", line 1, in <module> symbolic_prod(f(i), i, Integer(1), Integer(7)) File "/usr/local/sage8/local/lib/python2.7/sitepackages/sage/calculus/calculus.py", line 860, in symbolic_prod raise TypeError("need a multiplication variable") TypeError: need a multiplication variable ********************************************************************** File "src/sage/calculus/calculus.py", line 850, in sage.calculus.calculus.symbolic_prod Failed example: symbolic_prod(f(i), i, 1, n) Exception raised: Traceback (most recent call last): File "/usr/local/sage8/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 509, in _run self.compile_and_execute(example, compiler, test.globs) File "/usr/local/sage8/local/lib/python2.7/sitepackages/sage/doctest/forker.py", line 872, in compile_and_execute exec(compiled, globs) File "<doctest sage.calculus.calculus.symbolic_prod[7]>", line 1, in <module> symbolic_prod(f(i), i, Integer(1), n) File "/usr/local/sage8/local/lib/python2.7/sitepackages/sage/calculus/calculus.py", line 860, in symbolic_prod raise TypeError("need a multiplication variable") TypeError: need a multiplication variable ********************************************************************** File "src/sage/calculus/calculus.py", line 1489, in sage.calculus.calculus.laplace Failed example: laplace(t^n, t, s, algorithm='giac') Expected: Traceback (most recent call last): ... NotImplementedError: Unable to parse Giac output: integrate(t^n*exp(s*t),t,0,+infinity) Got: integration(t^n*e^(s*t), t, 0, +Infinity) ********************************************************************** 2 items had failures: 1 of 39 in sage.calculus.calculus.laplace 4 of 11 in sage.calculus.calculus.symbolic_prod [429 tests, 5 failures, 9.57 s]  sage t long src/sage/calculus/calculus.py # 5 doctests failed  Total time for all tests: 9.6 seconds cpu time: 9.1 seconds cumulative wall time: 9.6 seconds
The last one is identical to one already seen in 8.0.beta4 and 8.0.beta5 ; again, nothing new, else probably not related. The four first ones seem identical : aren't they related to an undeclared variable ?
 A couple suggestions, of varying importance :
 (major) the interface methods ("symbolic_expression.prod" (or product)) and function(s) ("symbolic_product", parallelling "symbolic_sum") are necessary for easy use of this new functionality.
 (minor) for aesthetics and consistency, could we have for sage.functions.other.symbolic_sum (aka Function_sum) (almost) the same _print_latex_ function you defined in sage.functions.other.symbolic_product (aka Function_prod) ?
 ((very) minor) consider a "mathematica" method, parallelling tye one in symbolic_sum (necessary for wouldbe Mathematica users, since mathematica.Sum(X(j), [j,1,p]) give utter nonsense currently).
 A question : can #22937 depend on this ?
 And a possible doctest, demonstrating that this formal mayhem has a mathematical sense :
sage: var("j,p", domain="integer") (j, p) sage: X=function("X") sage: sage.functions.other.symbolic_product(X(j),j,1,p).log().log_expand() sum(log(X(j)), j, 1, p)
==>needs_work
comment:13 Changed 6 years ago by
Commit:  e4769b5524ae5629db4189d06684646b22781efb → 58119b0df21bcfbb89fc58e108493fde90ac733f 

Branch pushed to git repo; I updated commit sha1. New commits:
58119b0  17505: fix doctests

comment:14 Changed 6 years ago by
Status:  needs_work → needs_review 

comment:15 followup: 16 Changed 6 years ago by
Ah sorry just a moment, I'll address your other issues ASAP.
comment:16 Changed 6 years ago by
comment:17 followup: 18 Changed 6 years ago by
Replying to charpent:
 (major) the interface methods ("symbolic_expression.prod" (or product))
Yes, but in a different ticket.
 A question : can #22937 depend on this ?
Yes you can state the dependency in the ticket form, but please wait with merging this branch, as I'll change the branch probably.
Agree to everything else.
comment:18 followup: 21 Changed 6 years ago by
Replying to rws:
Replying to charpent:
 (major) the interface methods ("symbolic_expression.prod" (or product))
Yes, but in a different ticket.
Okay. I'll take that in mind when implementing (and doctesting) #22937.
 A question : can #22937 depend on this ?
Yes you can state the dependency in the ticket form, but please wait with merging this branch, as I'll change the branch probably.
Okay also : I'll marl the dependency before starting the implementation of the multiplicative case. Should I wait your say so before reviewing the ticket ? Or should I review it in its current state ?
Suggestion : mark it as needs_work
before reworkiong it and needs_review
when satisfied with (a step of) your work.
Agree to everything else.
Thanks !
comment:19 Changed 6 years ago by
Commit:  58119b0df21bcfbb89fc58e108493fde90ac733f → 7a56004ca73a73e04b0b6a7bdbe6b528c0358bb3 

Branch pushed to git repo; I updated commit sha1. New commits:
7a56004  17505: address reviewer's suggestions

comment:21 followup: 22 Changed 6 years ago by
Replying to charpent:
Should I wait your say so before reviewing the ticket ? Or should I review it in its current state ?
Please review now, there may only be some doctests missing.
comment:22 Changed 6 years ago by
comment:23 followup: 24 Changed 6 years ago by
Status:  needs_review → positive_review 

Passes ptestlong
with the same failures already reported for 8.0.beta4 and not yet fixed.
===> positive review
Waiting your sayso to merge into #22937...
comment:24 Changed 6 years ago by
comment:27 followup: 30 Changed 6 years ago by
Slight issue :
sage: var("j,p", domain="integer") (j, p) sage: X=function("X") sage: latex(maxima("product(X(j),j,1,p)").sage()^2) \prod_{j=1}^{p} X(j)^{2}
which prints wrong (it should be {\prod_{j=1}^{p} X(j)}^{2
} in order to position the exponent over the whole sum, not the "productand"). The same is true for sum
.
Care to fix it here or should I open a new ticket (depending on this one) ?
comment:29 Changed 6 years ago by
Commit:  7a56004ca73a73e04b0b6a7bdbe6b528c0358bb3 → d420ec4ab00a322f97bd05326741ec7de3e3d65a 

Status:  positive_review → needs_review 
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
d420ec4  17505: fix latex, cosmetics

comment:30 followup: 31 Changed 6 years ago by
Replying to charpent:
which prints wrong (it should be
{\prod_{j=1}^{p} X(j)}^{2
} in order to position the exponent over the whole sum, not the "productand"). The same is true forsum
.
However, isn't it the duty of power to make the braces?
comment:31 Changed 6 years ago by
Replying to rws:
Replying to charpent:
which prints wrong (it should be
{\prod_{j=1}^{p} X(j)}^{2
} in order to position the exponent over the whole sum, not the "productand"). The same is true forsum
.However, isn't it the duty of power to make the braces?
Not necessarily. \sum_{j=1}^p X(j)^2
is mathematically wrong ; however, {X_{j}}^2
is correct but ugly, whereas X_{j}^2
is also ((almost) unambiguously) correct and much more pleasant.
There is still an ambiguity, that does not concern us there : tensors. But that's another whole can of worms.
Your patch looks good. ptestlong
is running and should terminate in about an hour.
comment:32 Changed 6 years ago by
Status:  needs_review → needs_work 

Two new failures :
sage t long src/sage/functions/other.py ********************************************************************** File "src/sage/functions/other.py", line 2617, in sage.functions.other.Function_ sum._print_latex_ Failed example: latex(ssum(x^2, x, 1, 10)) Expected: \sum_{x=1}^{10} x^2 Got: {\sum_{x=1}^{10} x^2} ********************************************************************** File "src/sage/functions/other.py", line 2664, in sage.functions.other.Function_prod._print_latex_ Failed example: latex(sprod(x^2, x, 1, 10)) Expected: \prod_{x=1}^{10} x^2 Got: {\prod_{x=1}^{10} x^2} ********************************************************************** 2 items had failures: 1 of 3 in sage.functions.other.Function_prod._print_latex_ 1 of 3 in sage.functions.other.Function_sum._print_latex_ [580 tests, 2 failures, 7.13 s]
It seems that you forgot to upate your doctests... ;)
==> needs work
(pro forma)
comment:33 Changed 6 years ago by
Commit:  d420ec4ab00a322f97bd05326741ec7de3e3d65a → 5779423aac340d1d7dc415da287b9af5e0e1f381 

Branch pushed to git repo; I updated commit sha1. New commits:
5779423  17505: fix doctests

comment:34 followup: 35 Changed 6 years ago by
Status:  needs_work → needs_review 

Hopefully my reviewers keep their patience. Thanks.
comment:35 Changed 6 years ago by
Replying to rws:
Hopefully my reviewers keep their patience. Thanks.
sage t long src/sage/symbolic/expression.pyx
passes with no error. ptestlong
underway (again, pro forma). Stay tuned.
comment:36 Changed 6 years ago by
Status:  needs_review → positive_review 

ptestlong
passes with the known, supposed unrelated failure ; no whoopee cushions.
==>positive_review
Since this was tested on top of #22937, the latter is also ready for rereview. Would you mind ?
comment:37 Changed 6 years ago by
Status:  positive_review → needs_work 

sr_prod
is missing a doctest.
comment:38 followup: 39 Changed 6 years ago by
Milestone:  sage8.0 → sageduplicate/invalid/wontfix 

Status:  needs_work → positive_review 
The ticket was merged up to a point. I'll close it and put the branch with the remaining issues in another ticket. See #22989.
comment:39 Changed 6 years ago by
comment:40 Changed 6 years ago by
Resolution:  → fixed 

Status:  positive_review → closed 
Sorry, forgot to close this ticket. Move your new commits to a new ticket.
Note that if #20179 is implemented it has to be adapted when symbolic products are made available.