Opened 3 years ago

Closed 2 years ago

#22989 closed defect (fixed)

Doctest: Remaining issues with symbolic product

Reported by: rws Owned by:
Priority: minor Milestone: sage-8.0
Component: calculus Keywords:
Cc: charpent, tscrim Merged in:
Authors: Ralf Stephan, Emmanuel Charpentier Reviewers: Marcelo Forets
Report Upstream: N/A Work issues:
Branch: 8be8a0c (Commits) Commit: 8be8a0cecbcda7aa7d75aeba104df3621cc70e87
Dependencies: #22937 Stopgaps:

Description

Continued from #17505 this ticket fixes LaTeX, documentation, and doctest issues around the symbolic product.

Change History (24)

comment:1 Changed 3 years ago by charpent

  • Cc charpent added

comment:2 Changed 3 years ago by rws

  • Cc charpent removed

BTW, Maxima can give back some "interesting" results:

sage: from sage.calculus.calculus import symbolic_product
sage: symbolic_product(1+(-1)^(x+1)/x,x,1,oo)            
...
ValueError: power::eval(): pow(1, Infinity) is not defined.

comment:3 Changed 3 years ago by rws

  • Cc charpent added

Oops.

comment:4 Changed 3 years ago by rws

  • Branch set to u/rws/22989

comment:5 follow-up: Changed 3 years ago by rws

  • Authors set to Ralf Stephan
  • Cc tscrim added
  • Commit set to e75c124b677884a65a8e7bade96fb678b81b9a56
  • Status changed from new to needs_review

New commits:

e75c12422989: Remaining issues with symbolic product

comment:6 in reply to: ↑ 5 Changed 3 years ago by charpent

Replying to rws:

New commits:

e75c12422989: Remaining issues with symbolic product

Can't pull this branch in mine : I've rebased on 8.0.beta6,and this one has a modification on src/sage/interfaces/maxima_lib.py incompatible with yours.

comment:7 Changed 3 years ago by rws

  • Branch changed from u/rws/22989 to u/rws/22989-1

comment:8 Changed 3 years ago by rws

  • Commit changed from e75c124b677884a65a8e7bade96fb678b81b9a56 to 7df31d9058169c3505a34f54f675663e4c2dcde7
  • Dependencies set to #22937

I ended up using your branch and resolve the conflict, so this depends on #22937.


Last 10 new commits:

687cc8eDistribute : implement Travis Scrimshaw's suggestion for iterations.
d420ec417505: fix latex, cosmetics
b784a2cMerge branch 'u/rws/implement_symbolic_product' of trac.sagemath.org:sage into distribute
577942317505: fix doctests
4b6e71bMerge branch 'u/rws/implement_symbolic_product' of trac.sagemath.org:sage into distribute
2412f7aDistribute : cosmetics on documentation.
c28097aDistribute : at his request, Travis Crimshaw removed from Author's list.
7aee739Distribute : one last typo (I hope...).
b2b4a0fMerge branch 'develop' into t/22937/distribute
7df31d922989: Remaining issues with symbolic product

comment:9 Changed 3 years ago by charpent

Found a small oversight in latex functions for Function_sum and Function_prod :

sage: latex(sum(sin(X(j)),j,1,p))
{\sum_{j=1}^{p} sin(X(j))}

whereas what is sought is something along the lines of {\sum_{j=1}^{p} \sin\left(X\left(j\right)\right)}

Patch suggestion :

charpent@asus16-ec:/usr/local/sage-8$ git diff
diff --git a/src/sage/functions/other.py b/src/sage/functions/other.py
index aaee96cf87..aafbb697e8 100644
--- a/src/sage/functions/other.py
+++ b/src/sage/functions/other.py
@@ -2617,7 +2617,8 @@ class Function_sum(BuiltinFunction):
             sage: latex(ssum(x^2, x, 1, 10))
             {\sum_{x=1}^{10} x^2}
         """
-        return r"{{\sum_{{{}={}}}^{{{}}} {}}}".format(var, a, b, x)
+        return r"{{\sum_{{{}={}}}^{{{}}} {}}}".format(latex(var), latex(a),
+                                                      latex(b), latex(x))
 
 symbolic_sum = Function_sum()
 
@@ -2664,6 +2665,7 @@ class Function_prod(BuiltinFunction):
             sage: latex(sprod(x^2, x, 1, 10))
             {\prod_{x=1}^{10} x^2}
         """
-        return r"{{\prod_{{{}={}}}^{{{}}} {}}}".format(var, a, b, x)
+        return r"{{\prod_{{{}={}}}^{{{}}} {}}}".format(latex(var), latex(a),
+                                                       latex(b), latex(x))
 
 symbolic_product = Function_prod()

Simple-minded but correct (I think).

comment:10 Changed 3 years ago by charpent

Yet another note :

sum() expands its first argument. I'm not sure that this is pertinent. Compare :

sage: sum((X(j)+Y(j))^2,j,1,p)
sum(X(j)^2 + 2*X(j)*Y(j) + Y(j)^2, j, 1, p)
sage: maxima("sum(((X(j)+Y(j))^2),j,1,p)").sage()
sum((X(j) + Y(j))^2, j, 1, p)

I have implemented (in a private branch), an "expand" option controlling if distribute() should expand its first argument (default) or not (might come in handy in some situations). This allows :

sage: maxima("sum((X(j)+Y(j))^2+Z(j),j,1,p)").sage().distribute()
sum(X(j)^2, j, 1, p) + sum(2*X(j)*Y(j), j, 1, p) + sum(Y(j)^2, j, 1, p) + sum(Z(j), j, 1, p)
sage: maxima("sum((X(j)+Y(j))^2+Z(j),j,1,p)").sage().distribute(expand=False)
sum((X(j) + Y(j))^2, j, 1, p) + sum(Z(j), j, 1, p)

But this currently works only from sum expressions cast from Maxima ; Sage-built sums get expanded volens nolens, as seen above, and the resulting expansions can't be factorized back by factor().

Possible solution : an "expand" keyword argument to sum (default=True) controlling the expansion ? What do you think ?

The same goes, of course, for products.

comment:11 Changed 3 years ago by chapoton

Please take take ASAP of the apply/python3 issue introduced in #22937.

comment:12 Changed 3 years ago by rws

  • Summary changed from Remaining issues with symbolic product to Doctest: Remaining issues with symbolic product

The branch is fine.

comment:13 Changed 3 years ago by charpent

  • Branch changed from u/rws/22989-1 to u/charpent/22989-1

comment:14 Changed 3 years ago by charpent

  • Authors changed from Ralf Stephan to Ralf Stephan, Emmanuel Charpentier
  • Commit changed from 7df31d9058169c3505a34f54f675663e4c2dcde7 to 4c3d7f478979aaa6bc72aae57e57a0209b9942fb

This has been rebased over 8.0.beta7 (which incorporates #22937). Three fixes :

  • Latex generation for symbolic sums and products.
  • A .prod() method for symbolic expressions.
  • A public product() function (I wanted to nmae it prod, but this clashes irreconciliably with the existing prod function for lists and others. I you see how to implement this, you're welcome...).

Passes ptestlong with the usual transient sage -t --long src/sage/homology/simplicial_complex.py failure, which is transient.

==> needs_review

comment:15 Changed 3 years ago by charpent

As of the day before yesterday, the patchbots started giving an error in building g2fx that I don't understand a bit...

Ca some king sould enlighten me on the possible causes (and possible remedies ?)

Version 0, edited 3 years ago by charpent (next)

comment:16 follow-up: Changed 3 years ago by mforets

some comments:

  • the (optional) use Giac should be just use Giac
  • 'mathematica' - (optional) use Mathematica is missing
  • add a SEEALSO block pointing to the new symbolic_product in the top level prod (misc_c.pyx)

if you don't mind, i can add these minor things myself and review asap.

comment:17 in reply to: ↑ 16 ; follow-up: Changed 3 years ago by charpent

Replying to mforets:

some comments:

  • the (optional) use Giac should be just use Giac

Indeed : giac is now standard...

  • 'mathematica' - (optional) use Mathematica is missing

Indeed.

But I'm not so sure : the Mathematica interface has other (serious) problems, that can be triggered also in sums and products. This, IMHO, is a distinct problem, and should be fixed by someone knowing what it does with Mathematica (I don't...).

Is it reasonable do document a (good) way to use a (flaky) interface ? I let you judge...

  • add a SEEALSO block pointing to the new symbolic_product in the top level prod (misc_c.pyx)

Right...

if you don't mind, i can add these minor things myself and review asap.

Please go ahead ! Do you need me to review your review ?

comment:18 Changed 3 years ago by mforets

  • Branch changed from u/charpent/22989-1 to u/mforets/22989-1

comment:19 Changed 3 years ago by mforets

  • Commit changed from 4c3d7f478979aaa6bc72aae57e57a0209b9942fb to 5b8b16c47fb0d47835fe1e6c69465d051e4ba252

done. for mathematica unfortunately i also don't have it so cannot test, but i do think it's good to mention as a valid option.

i'm having an issue with the hold option:

sage: k, n = var('k, n')
sage: sage.calculus.calculus.symbolic_product(k, k, 1, n)
factorial(n)
sage: sage.calculus.calculus.symbolic_product(k, k, 1, n, hold=True)
---------------------------------------------------------------------------
ImportError                               Traceback (most recent call last)
<ipython-input-3-1fc22673b8c7> in <module>()
----> 1 sage.calculus.calculus.symbolic_product(k, k, Integer(1), n, hold=True)

/Users/forets/sage-src/sage/local/lib/python2.7/site-packages/sage/calculus/calculus.pyc in symbolic_product(expression, v, a, b, algorithm, hold)
    868
    869     if hold == True:
--> 870         from sage.functions.other import symbolic_prod as sprod
    871         return sprod(expression, v, a, b)
    872

ImportError: cannot import name symbolic_prod

let me fix it by changing symbolic_prod to symbolic_product in line 870


New commits:

5b8b16cdocstring tweaks

comment:20 in reply to: ↑ 17 Changed 3 years ago by tscrim

Replying to charpent:

Replying to mforets:

  • 'mathematica' - (optional) use Mathematica is missing

But I'm not so sure : the Mathematica interface has other (serious) problems, that can be triggered also in sums and products. This, IMHO, is a distinct problem, and should be fixed by someone knowing what it does with Mathematica (I don't...).

Is it reasonable do document a (good) way to use a (flaky) interface ? I let you judge...

We are suppose to be supporting an interface to Mathematica, so I think we should document it. Doing so will both increase our user base and help find bugs from people using said interface.

comment:21 Changed 3 years ago by git

  • Commit changed from 5b8b16c47fb0d47835fe1e6c69465d051e4ba252 to 8be8a0cecbcda7aa7d75aeba104df3621cc70e87

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

8be8a0cfix import in hold option

comment:22 Changed 3 years ago by mforets

  • Reviewers set to Marcelo Forets
  • Status changed from needs_review to positive_review

symbolic product works and html doc builds, tests in relevant modules pass.

comment:23 Changed 3 years ago by rws

Thanks.

comment:24 Changed 2 years ago by vbraun

  • Branch changed from u/mforets/22989-1 to 8be8a0cecbcda7aa7d75aeba104df3621cc70e87
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.