Do not check for a zero result in arithmetics of coordinate functions and scalar fields

[egourgoulhon]

In the arithmetics of symbolic coordinate functions (methods sage.manifolds.coord_func_symb.CoordFunctionSymb._add_, _sub_, _mul_ and _div_), the final result (res below) is checked against zero as follows:

if res == 0:
    return self.parent().zero()

Now, for complicated expressions, such a check involves Maxima and is very slow. Moreover, there is no real benefit in representing all the zero functions by a unique object (the zero of the function ring). This ticket proposes therefore to remove the check.
It also removes a similar check in the arithmetics of scalar fields (class sage.manifolds.scalarfield.ScalarField).

The gain in performance is quite significant:
sage -tp --long src/sage/manifolds/ (with 8 cores) yields

Total time for all tests: 245.9 seconds
    cpu time: 1570.5 seconds
    cumulative wall time: 1689.6 seconds

instead of

Total time for all tests: 300.5 seconds
    cpu time: 1757.9 seconds
    cumulative wall time: 1876.0 seconds

On computationally demanding worksheets, like the Kerr spacetime one, the gain is even greater: the total time for running all cells is 386 s, instead of 610 s.

Besides, the method is_zero of class CoordFunctionSymb is replaced by a method __bool__ (with an alias to __nonzero__ for Python 2 compatibility), leaving the method is_zero only in the base class sage.structure.element.Element.

CC: @tscrim

Component: geometry

Keywords: coordinate function, scalar field

Author: Eric Gourgoulhon

Branch/Commit: 94e8c68

Reviewer: Travis Scrimshaw

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

[egourgoulhon]

Branch: public/manifolds/coord_function_arithm-22859

[egourgoulhon]

New commits:

0960e64

Do not check for a zero result in arithmetics of coordinate functions and scalar fields

[egourgoulhon]

Commit: 0960e64

[egourgoulhon]

Description changed:

--- 
+++ 
@@ -1,4 +1,4 @@
-In the arithmetics of symbolic coordinate functions (methods `sage.manifolds.coord_func_symb.CoordFunctionSymb._add_`, `_sub_`, `_mul_` and `div`), the final result (`res` below) is checked against zero as follows:
+In the arithmetics of symbolic coordinate functions (methods `sage.manifolds.coord_func_symb.CoordFunctionSymb._add_`, `_sub_`, `_mul_` and `_div_`), the final result (`res` below) is checked against zero as follows:
 
 ```
 if res == 0:

[egourgoulhon]

Description changed:

--- 
+++ 
@@ -4,7 +4,7 @@
 if res == 0:
     return self.parent().zero()
 ```
-Now, for complicated expressions, such a check involves Maxima and is very slow. Moreover, there is no great benefit in representing all the zero functions by a unique object (the zero of the parent). This tickets proposes therefore to remove the check. 
+Now, for complicated expressions, such a check involves Maxima and is very slow. Moreover, there is no real benefit in representing all the zero functions by a unique object (the zero of the function ring). This ticket proposes therefore to remove the check. 
 It also removes a similar check in the arithmetics of scalar fields (class `sage.manifolds.scalarfield.ScalarField`).
 
 The gain in performance is quite significant:

[tscrim]

comment:4

The question comes down to checking whether the element represents zero or if constructing a new 0 element is faster. There is also the x.is_trivial_zero(), which might catch some of the cases. Additionally, am I correct is that is_zero returns not self? Otherwise it LGTM.

[sagetrac-git]

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

94e8c68

Use of is_trivial_zero in arithmetics of coordinate functions

[sagetrac-git]

Changed commit from 0960e64 to 94e8c68

[egourgoulhon]

comment:6

Replying to @tscrim:

The question comes down to checking whether the element represents zero or if constructing a new 0 element is faster.

Indeed.

There is also the x.is_trivial_zero(), which might catch some of the cases.

Thanks for pointing out is_trivial_zero(); I've restored checks by means of this function in the above commit. For sage -tp --long src/sage/manifolds/, there is no noticeable difference and for the Kerr metric worksheet, the total time becomes 378 s, instead of 386 s. So I think we can buy it!

Additionally, am I correct is that is_zero returns not self?

Yes, it is defined like this in src/sage/structure/element.pyx and it is that method which is inheritated by coordinate functions.