Opened 12 years ago

Closed 7 years ago

# Provide symbolic sum function with evalf

Reported by: Owned by: Burcin Erocal Burcin Erocal major sage-6.8 symbolics Wilfried Huss, Karl-Dieter Crisman, Eviatar Bach Ralf Stephan Daniel Krenn N/A 4f7b161 4f7b1610a3fd78eeea2e188605bccf3983a76490 #17759

Symbolics sums returned from maxima cannot be numerically evaluated, since they don't define an `_evalf_()` method.

This was reported by dirkd on sage-support:

```Why is evaluating this expression problematical?

y1(x)=x^2;y2(x)=5-x;
a0=1;an=3;Delta=(an-a0)/n;p(k)=a0+(k-1/2)*Delta;
I(n)=sum(abs(y2(p(k))-y1(p(k)))*Delta,k,1,n);
N(I(10))

SAGE respons:
<snipped traceback>
File "expression.pyx", line 3797, in
sage.symbolic.expression.Expression.n (sage/symbolic/expression.cpp:
17022)
TypeError: cannot evaluate symbolic expression numerically
```

### comment:4 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.11 → sage-5.12

### comment:5 Changed 9 years ago by For batch modifications

Milestone: sage-6.1 → sage-6.2

### comment:6 Changed 9 years ago by For batch modifications

Milestone: sage-6.2 → sage-6.3

### comment:7 Changed 8 years ago by Ralf Stephan

Milestone: sage-6.3 → sage-duplicate/invalid/wontfix new → needs_review

Your problem is threefold: 1. you use `n` both as sum endpoint and variable, 2. Sage `sum` is only intended with symbolic endpoints, and 3. the need to use Python summation may require defining a Python function instead of a Sage symbolic function:

```sage: sum?
...
Warning: This function only works with symbolic expressions. To sum any
other objects like list elements or function return values,

sage: I(n)=sum(abs(y2(p(k))-y1(p(k)))*Delta,k,1,n);
sage: I
n |--> 2*sum(abs(-4*k^2 - 3*(2*k - 1)*n + 3*n^2 + 4*k - 1), k, 1, n)/n^3

sage: def I(n):
....:     return (2*sum(abs(-4*k^2 - 3*(2*k - 1)*n + 3*n^2 + 4*k - 1) for k in range(1,n+1))/n^3)
....:
sage: I(10)
1301/250
sage: [I(i) for i in range(1,11)]
[2, 5, 46/9, 5, 26/5, 277/54, 254/49, 665/128, 418/81, 1301/250]
```

### comment:8 Changed 8 years ago by Karl-Dieter Crisman

Milestone: sage-duplicate/invalid/wontfix → sage-6.3 defect → enhancement

I don't think any of these invalidate the ticket; the point is to extend the behavior. Why is 1. a problem? This seems like it should be a nice function to me. See Burcin's reply in the thread:

```> If I leave out the N( )-operator on the last line the block evaluates
> to
>
>
> 1/500*sum(abs(-4*k^2 - 56*k + 329), k, 1, 10)
>
> which can be evaluated in a new inputbox. Why not in one step?
The result returned from maxima uses a symbolic function object created
on the fly. This is quite different from the sum() function
available on the command line, and unfortunately, it doesn't define a
numerical evaluation function, _evalf_().
```

Burcin knows this code very well, so I would be surprised if he misdiagnosed this. But I figure maybe changing to enhancement will appease everyone :)

### comment:9 Changed 8 years ago by Nils Bruin

Burcin is spot-on:

```sage: S=(I(10).operands()[0].operator()); S
sum
sage: type(S)
<class 'sage.symbolic.function_factory.NewSymbolicFunction'>
```

(Note the "New", not "BuiltIn?" or similar. It's a completely generic placeholder) we just need a symbolic function hooked up that can do some mildly intelligent evaluation when asked for it.

Incidentally, we can just map back to maxima and do the right thing there:

```sage: maxima_calculus(I(10))
('sum(abs(4*_SAGE_VAR_k^2+56*_SAGE_VAR_k-329),_SAGE_VAR_k,1,10))/500
sage: SR(maxima_calculus(I(10)).simplify_sum())
1301/250
```

(I haven't checked if it's correct). You can see why the "simplify_sum" is required: the newly created "sum" function in SR is linked to the inert "'sum".

### comment:10 Changed 8 years ago by Ralf Stephan

Status: needs_review → needs_work

### comment:11 Changed 8 years ago by For batch modifications

Milestone: sage-6.3 → sage-6.4

### comment:12 follow-up:  13 Changed 8 years ago by Karl-Dieter Crisman

Maybe it's time we fixed this.

### comment:13 in reply to:  12 Changed 8 years ago by Ralf Stephan

Description: modified (diff) numerical evaluation of symbolic sums → FP evaluation of symbolic sums fails

Maybe it's time we fixed this.

It is not clear if forcing people to use `N()` and getting a float, even if there is an integer simplification of the sum, is the right thing to do. Granted, the error thrown on `N(I(10))` is a bug, and this ticket is about it. Here is a minimal example:

```sage: (k,n) = var('k,n')
sage: f(n)=sum(abs(-k*k+n),k,1,n)
sage: f(n=8)
sum(abs(-k^2 + 8), k, 1, 8)
sage: N(f(8))
```

However, I would expect `f(n=8).simplify()` or `.expand()` to give me the result `162`, and this is #17422

### comment:14 Changed 8 years ago by Ralf Stephan

Description: modified (diff) enhancement → defect

### comment:15 Changed 8 years ago by Ralf Stephan

Status: needs_work → needs_info

As #15346 would implement `simplify_sum`, what remains for this ticket to do?

### comment:16 Changed 8 years ago by Karl-Dieter Crisman

Summary: FP evaluation of symbolic sums fails → Provide symbolic sum function with evalf

As #15346 would implement simplify_sum, what remains for this ticket to do?

I suppose there is the issue that one might not want to have to simplify in order to get a floating-point number. Really, we shouldn't require that. With #15346 one needs

```sage: I(10).expand_sum().n()
```

but really `I(10).n()` should suffice, and for that we need a symbolic sum function. I'll clarify the title, though, as you are right that this is at least now much more possible. I'll probably add the example as a reviewer patch on #15346, in fact.

### comment:17 Changed 8 years ago by Karl-Dieter Crisman

Status: needs_info → needs_work

### comment:18 Changed 8 years ago by Ralf Stephan

This is a proof of concept-patch that works, but only with bare sums. I guess the walk on the expression tree happens somewhere in Pynac.

```diff --git a/src/sage/symbolic/expression.pyx b/src/sage/symbolic/expression.pyx
--- a/src/sage/symbolic/expression.pyx
+++ b/src/sage/symbolic/expression.pyx
@@ -4847,6 +4847,19 @@ cdef class Expression(CommutativeRingElement):
sage: all(len(str(e.n(digits=k)))-1 >= k for k in ks)
True

+        We also allow evaluation of unexpanded symbolic sums
+        with numeric limits (:trac:`9424`)::
+
+            sage: (k,n) = var('k,n')
+            sage: f(n) = sum(abs(-k*k+n), k, 1, n)
+            sage: f(n=8)
+            sum(abs(-k^2 + 8), k, 1, 8)
+            sage: f(n=8).n()
+            162.0
+            sage: f(n=x).n()
+            Traceback (most recent call last):
+            ...
+            TypeError: cannot evaluate symbolic expression numerically
"""
if prec is None:
if digits is None:
@@ -4865,11 +4878,16 @@ cdef class Expression(CommutativeRingElement):
x = self._convert(kwds)

# we have to consider constants as well, since infinity is a constant
-        # in pynac
+        # in pynac. Also, catch symbolic sums with numeric limits.
if is_a_numeric(x._gobj):
res = py_object_from_numeric(x._gobj)
-        elif  is_a_constant(x._gobj):
+        elif is_a_constant(x._gobj):
res = x.pyobject()
+        elif x.operator().name() == 'sum' and (
+                    is_a_numeric((<Expression>x.operands()[2])._gobj)
+                and is_a_numeric((<Expression>x.operands()[3])._gobj)):
+            from sage.calculus.calculus import symbolic_sum
+            return symbolic_sum(*(x.operands()))
else:
raise TypeError("cannot evaluate symbolic expression numerically")
```

but

```sage: (k,n) = var('k,n')
sage: f(n) = sum(abs(-k*k+n),k,1,n)
sage: 1+f(n=8)
sum(abs(-k^2 + 8), k, 1, 8) + 1
sage: _.n()
BOOM
```

### comment:19 Changed 8 years ago by Ralf Stephan

Branch: → u/rws/provide_symbolic_sum_function_with_evalf

### comment:20 Changed 8 years ago by Ralf Stephan

Authors: → Ralf Stephan → 32bca6ab2b1cd43e92fb3bdc262c93a9e8ca6540 → #17759 needs_work → needs_review

I think I found a satisfying solution.

New commits:

 ​3781eec `17759: convenience class symbolic ExpressionTreeWalker(Converter)` ​32bca6a `9424: Provide symbolic sum function with evalf`

### comment:21 follow-up:  22 Changed 8 years ago by Karl-Dieter Crisman

You'd think that given the massive amounts of snow here lately I'd have time to carefully look this over - but I don't :-(

However I think this looks like a good start at a solution, and definitely the first commit is an improvement in any case - perhaps one that deserves its own ticket, for quicker review? (There might be more people interested in that, honestly.) Unless someone complained with the name change - but it was never globally imported and frankly we do need a general expression tree walker more easily accessible, see #9329 for instance. So connecting this (not here, maybe) to the `Converter` class it inherits from and explaining when one would use each one would be useful too.

The main concern I have in terms of the main purpose of the ticket is to adequately test and debug `DefiniteSumExpander`, especially since it is in the code in such a way that it isn't doctested (other than the case in question).

### comment:22 in reply to:  21 ; follow-up:  23 Changed 8 years ago by Ralf Stephan

However I think this looks like a good start at a solution, and definitely the first commit is an improvement in any case - perhaps one that deserves its own ticket, for quicker review?

I don't understand. You did see the number #17759?

The main concern I have in terms of the main purpose of the ticket is to adequately test and debug `DefiniteSumExpander`, especially since it is in the code in such a way that it isn't doctested (other than the case in question).

Most of it is doctested in #17759, i.e., its base class.

### comment:23 in reply to:  22 ; follow-up:  24 Changed 8 years ago by Karl-Dieter Crisman

However I think this looks like a good start at a solution, and definitely the first commit is an improvement in any case - perhaps one that deserves its own ticket, for quicker review?

I don't understand. You did see the number #17759?

Nope! I really didn't have a lot of time - never even saw the dependency field. Maybe I shouldn't look at tickets after shoveling ;)

Most of it is doctested in #17759, i.e., its base class.

I did see that doctesting, I just would want to see what was different in the sum case.

I'll keep these on the front burner.

### comment:24 in reply to:  23 Changed 8 years ago by Ralf Stephan

I did see that doctesting, I just would want to see what was different in the sum case.

There the `composition` method is overwritten (and tested).

### comment:25 Changed 8 years ago by Ralf Stephan

Status: needs_review → needs_work
```sage -t --long src/sage/functions/hypergeometric.py  # 11 doctests failed
sage -t --long src/sage/functions/trig.py  # 1 doctest failed
```

### comment:26 Changed 8 years ago by Ralf Stephan

Fixing these doctests will need the solutions found for fixing #17849 (or vice versa).

### comment:27 Changed 8 years ago by git

Commit: 32bca6ab2b1cd43e92fb3bdc262c93a9e8ca6540 → 27a16927956651abf9361d8c0a12dffb00cba963

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

 ​5a1ac7e `Merge branch 'develop' into t/9424/provide_symbolic_sum_function_with_evalf` ​9375510 `Merge branch 'develop' into t/17759/public/17759` ​16aa81d `17759: handle hold=True and hypergeometric` ​1f4edf2 `Merge branch 'public/17759' of trac.sagemath.org:sage into t/9424/provide_symbolic_sum_function_with_evalf` ​27a1692 `9424: delegate to superclass; fix doctest`

### comment:28 Changed 8 years ago by Ralf Stephan

Milestone: sage-6.4 → sage-6.6 needs_work → needs_review

### comment:29 Changed 8 years ago by Ralf Stephan

Branch: u/rws/provide_symbolic_sum_function_with_evalf → u/rws/9424

### comment:30 Changed 8 years ago by Ralf Stephan

Commit: 27a16927956651abf9361d8c0a12dffb00cba963 → b213d69bf13e1c7e21d2da2868bfd7908f5d31eb sage-6.6 → sage-6.8

New commits:

 ​fa73285 `17759: convenience class symbolic ExpressionTreeWalker(Converter)` ​b213d69 `Merge branch 'u/rws/provide_symbolic_sum_function_with_evalf' of trac.sagemath.org:sage into tmp2`

### comment:31 Changed 8 years ago by Marc Mezzarobba

Status: needs_review → needs_work

`TestsFailed` (says the patchbot)

### comment:32 Changed 7 years ago by git

Commit: b213d69bf13e1c7e21d2da2868bfd7908f5d31eb → 4f7b1610a3fd78eeea2e188605bccf3983a76490

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

 ​985fdb4 `Merge branch 'develop' into t/9424/9424` ​4f7b161 `9424: make ceil/floor accept kwds; fix doctests`

### comment:33 Changed 7 years ago by Ralf Stephan

Status: needs_work → needs_review

### comment:34 Changed 7 years ago by Daniel Krenn

Reviewers: → Daniel Krenn needs_review → positive_review

LGTM

### comment:35 Changed 7 years ago by Volker Braun

Branch: u/rws/9424 → 4f7b1610a3fd78eeea2e188605bccf3983a76490 → fixed positive_review → closed
Note: See TracTickets for help on using tickets.