sagemath / sage

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

Doctest: Remaining issues with symbolic product #22989

Closed rwst closed 7 years ago

rwst commented 7 years ago

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

Depends on #22937

CC: @EmmanuelCharpentier @tscrim

Component: calculus

Author: Ralf Stephan, Emmanuel Charpentier

Branch/Commit: 8be8a0c

Reviewer: Marcelo Forets

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

rwst commented 7 years ago
comment:2

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.
rwst commented 7 years ago
comment:3

Oops.

rwst commented 7 years ago

Branch: u/rws/22989

rwst commented 7 years ago

New commits:

e75c12422989: Remaining issues with symbolic product
rwst commented 7 years ago

Author: Ralf Stephan

rwst commented 7 years ago

Commit: e75c124

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 7 years ago
comment:6

Replying to @rwst:

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.

rwst commented 7 years ago

Changed branch from u/rws/22989 to u/rws/22989-1

rwst commented 7 years ago
comment:8

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
rwst commented 7 years ago

Dependencies: #22937

rwst commented 7 years ago

Changed commit from e75c124 to 7df31d9

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 7 years ago
comment:9

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

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 7 years ago
comment:10

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.

fchapoton commented 7 years ago
comment:11

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

rwst commented 7 years ago
comment:12

The branch is fine.

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 7 years ago

Changed branch from u/rws/22989-1 to u/charpent/22989-1

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 7 years ago

Changed author from Ralf Stephan to Ralf Stephan, Emmanuel Charpentier

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 7 years ago

Changed commit from 7df31d9 to 4c3d7f4

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 7 years ago
comment:14

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

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

==> needs_review

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 7 years ago
comment:15

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

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

c22b6800-ec0b-4cbf-94c4-0a74aecc2093 commented 7 years ago
comment:16

some comments:

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

7822f248-ba56-45f1-ab3d-4de7482bdf9f commented 7 years ago
comment:17

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 ?

c22b6800-ec0b-4cbf-94c4-0a74aecc2093 commented 7 years ago

Changed branch from u/charpent/22989-1 to u/mforets/22989-1

c22b6800-ec0b-4cbf-94c4-0a74aecc2093 commented 7 years ago

Changed commit from 4c3d7f4 to 5b8b16c

c22b6800-ec0b-4cbf-94c4-0a74aecc2093 commented 7 years ago
comment:19

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
tscrim commented 7 years ago
comment:20

Replying to @EmmanuelCharpentier:

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.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

Changed commit from 5b8b16c to 8be8a0c

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 7 years ago

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

8be8a0cfix import in hold option
c22b6800-ec0b-4cbf-94c4-0a74aecc2093 commented 7 years ago
comment:22

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

c22b6800-ec0b-4cbf-94c4-0a74aecc2093 commented 7 years ago

Reviewer: Marcelo Forets

rwst commented 7 years ago
comment:23

Thanks.

vbraun commented 7 years ago

Changed branch from u/mforets/22989-1 to 8be8a0c