Opened 5 years ago

Closed 2 years ago

#17505 closed enhancement (fixed)

implement symbolic product

Reported by: rws Owned by:
Priority: major Milestone: sage-duplicate/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 rws)

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)
    <ipython-input-5-85e69544cbe9> in <module>()
    ----> 1 prod(X(j),j,Integer(1),p)
    
    /usr/local/sage-8/src/sage/misc/misc_c.pyx in sage.misc.misc_c.prod (/usr/local/sage-8/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)
    <ipython-input-6-4d04d74c7489> 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 5 years ago by rws

  • Description modified (diff)

comment:2 Changed 4 years ago by rws

  • Milestone changed from sage-6.5 to sage-wishlist

comment:3 Changed 2 years ago by rws

Note that if #20179 is implemented it has to be adapted when symbolic products are made available.

comment:4 Changed 2 years ago by charpent

  • Cc charpent added

comment:5 follow-up: Changed 2 years ago by charpent

  • Description modified (diff)

Cut'n paste of the description of #22914 (duplicate ticket), at the request of the present ticket's author.

Note : Couldn't we cut'n'paste the recent code for symbolic sums (#21645) ?

comment:6 in reply to: ↑ 5 Changed 2 years ago by rws

Replying to charpent:

Note : Couldn't we cut'n'paste the recent code for symbolic sums (#21645) ?

Yes, but as you can see with #22844 it may not work 100%.

comment:7 Changed 2 years ago by rws

  • Description modified (diff)
  • Milestone changed from sage-wishlist to sage-8.0

comment:8 Changed 2 years ago by rws

  • Branch set to u/rws/implement_symbolic_product

comment:9 Changed 2 years ago by mforets

  • Commit set to 647ff396cc17c8acb59043dad905ab882834017b

do you mind adding giac='product' ?

since:

sage: giac('product(x, x, 1, n)')
n!
sage: _.sage()
factorial(n)

New commits:

647ff3917505: unevaluated symbolic product
Last edited 2 years ago by mforets (previous) (diff)

comment:10 Changed 2 years ago by git

  • Commit changed from 647ff396cc17c8acb59043dad905ab882834017b to e4769b5524ae5629db4189d06684646b22781efb

Branch pushed to git repo; I updated commit sha1. New commits:

e4769b517505: symbolic product

comment:11 follow-up: Changed 2 years ago by rws

  • Authors set to Ralf Stephan
  • Status changed from new to 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 in reply to: ↑ 11 ; follow-up: Changed 2 years ago by charpent

  • Status changed from needs_review to 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 high-school/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@asus16-ec:/usr/local/sage-8$ sage -t --long src/sage/calculus/calculus.py
too many failed tests, not using stored timings
Running doctests with ID 2017-05-08-14-44-39-93705f01.
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/sage-8/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 509, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/local/sage-8/local/lib/python2.7/site-packages/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/sage-8/local/lib/python2.7/site-packages/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/sage-8/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 509, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/local/sage-8/local/lib/python2.7/site-packages/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/sage-8/local/lib/python2.7/site-packages/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/sage-8/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 509, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/local/sage-8/local/lib/python2.7/site-packages/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/sage-8/local/lib/python2.7/site-packages/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/sage-8/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 509, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/usr/local/sage-8/local/lib/python2.7/site-packages/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/sage-8/local/lib/python2.7/site-packages/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 :
  1. (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.
  1. (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) ?
  1. ((very) minor) consider a "mathematica" method, parallelling tye one in symbolic_sum (necessary for would-be 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 2 years ago by git

  • Commit changed from e4769b5524ae5629db4189d06684646b22781efb to 58119b0df21bcfbb89fc58e108493fde90ac733f

Branch pushed to git repo; I updated commit sha1. New commits:

58119b017505: fix doctests

comment:14 Changed 2 years ago by rws

  • Status changed from needs_work to needs_review

comment:15 follow-up: Changed 2 years ago by rws

Ah sorry just a moment, I'll address your other issues ASAP.

comment:16 in reply to: ↑ 15 Changed 2 years ago by charpent

Replying to rws:

Ah sorry just a moment, I'll address your other issues ASAP.

Those were just suggestions, not requests ;-)

Unless you have other suggestions, I'll implement the "symbolic product of product" case in #22937 ASAP, and depend on the present ticket.

comment:17 in reply to: ↑ 12 ; follow-up: Changed 2 years ago by rws

Replying to charpent:

  1. (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 in reply to: ↑ 17 ; follow-up: Changed 2 years ago by charpent

Replying to rws:

Replying to charpent:

  1. (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 2 years ago by git

  • Commit changed from 58119b0df21bcfbb89fc58e108493fde90ac733f to 7a56004ca73a73e04b0b6a7bdbe6b528c0358bb3

Branch pushed to git repo; I updated commit sha1. New commits:

7a5600417505: address reviewer's suggestions

comment:20 Changed 2 years ago by rws

OK, which doctests are still missing?

comment:21 in reply to: ↑ 18 ; follow-up: Changed 2 years ago by rws

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 in reply to: ↑ 21 Changed 2 years ago by charpent

Replying to rws:

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.

Builds OK on top of my distribute branch. ptestlon running (needs about 1 hour : stay tuned...).

comment:23 follow-up: Changed 2 years ago by charpent

  • Status changed from needs_review to positive_review

Passes ptestlong with the same failures already reported for 8.0.beta4 and not yet fixed.

===> positive review

Waiting your say-so to merge into #22937...

comment:24 in reply to: ↑ 23 Changed 2 years ago by rws

Thanks.

Replying to charpent:

Waiting your say-so to merge into #22937...

Please go ahead.

comment:25 Changed 2 years ago by rws

Your real name in the Reviewer field is missing, please add.,

comment:26 Changed 2 years ago by charpent

  • Reviewers set to Emmanuel Charpentier

Wups !

Done...

comment:27 follow-up: Changed 2 years ago by charpent

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) ?

Last edited 2 years ago by charpent (previous) (diff)

comment:28 Changed 2 years ago by rws

I'll fix it immediately.

comment:29 Changed 2 years ago by git

  • Commit changed from 7a56004ca73a73e04b0b6a7bdbe6b528c0358bb3 to d420ec4ab00a322f97bd05326741ec7de3e3d65a
  • Status changed from positive_review to needs_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

d420ec417505: fix latex, cosmetics

comment:30 in reply to: ↑ 27 ; follow-up: Changed 2 years ago by 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 for sum.

However, isn't it the duty of power to make the braces?

comment:31 in reply to: ↑ 30 Changed 2 years ago by charpent

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 for sum.

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 2 years ago by charpent

  • Status changed from needs_review to 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 2 years ago by git

  • Commit changed from d420ec4ab00a322f97bd05326741ec7de3e3d65a to 5779423aac340d1d7dc415da287b9af5e0e1f381

Branch pushed to git repo; I updated commit sha1. New commits:

577942317505: fix doctests

comment:34 follow-up: Changed 2 years ago by rws

  • Status changed from needs_work to needs_review

Hopefully my reviewers keep their patience. Thanks.

comment:35 in reply to: ↑ 34 Changed 2 years ago by charpent

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 2 years ago by charpent

  • Status changed from needs_review to 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 re-review. Would you mind ?

comment:37 Changed 2 years ago by tscrim

  • Status changed from positive_review to needs_work

sr_prod is missing a doctest.

comment:38 follow-up: Changed 2 years ago by rws

  • Milestone changed from sage-8.0 to sage-duplicate/invalid/wontfix
  • Status changed from needs_work to 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 in reply to: ↑ 38 Changed 2 years ago by charpent

Replying to rws:

The ticket was merged up to a point.

You mean in #22937, I suppose ?

I'll close it and put the branch with the remaining issues in another ticket. See #22989.

Hmm... Unles I'm mistaken, there are two consequences :

Am I right ?

comment:40 Changed 2 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed

Sorry, forgot to close this ticket. Move your new commits to a new ticket.

Note: See TracTickets for help on using tickets.