Opened 22 months ago
Closed 22 months ago
#22859 closed enhancement (fixed)
Do not check for a zero result in arithmetics of coordinate functions and scalar fields
Reported by:  egourgoulhon  Owned by:  

Priority:  major  Milestone:  sage8.0 
Component:  geometry  Keywords:  coordinate function, scalar field, manifold 
Cc:  tscrim  Merged in:  
Authors:  Eric Gourgoulhon  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  94e8c68 (Commits)  Commit:  94e8c68536ffc0027c6f82177b10140dc43e58ef 
Dependencies:  Stopgaps: 
Description (last modified by )
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
.
Change History (9)
comment:1 Changed 22 months ago by
 Branch set to public/manifolds/coord_function_arithm22859
 Commit set to 0960e64cbaf86ba418716fed4e41384c948c4eed
 Status changed from new to needs_review
comment:2 Changed 22 months ago by
 Description modified (diff)
comment:3 Changed 22 months ago by
 Cc tscrim added
 Description modified (diff)
comment:4 followup: ↓ 6 Changed 22 months ago by
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.
comment:5 Changed 22 months ago by
 Commit changed from 0960e64cbaf86ba418716fed4e41384c948c4eed to 94e8c68536ffc0027c6f82177b10140dc43e58ef
Branch pushed to git repo; I updated commit sha1. New commits:
94e8c68  Use of is_trivial_zero in arithmetics of coordinate functions

comment:6 in reply to: ↑ 4 Changed 22 months ago by
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
returnsnot self
?
Yes, it is defined like this in src/sage/structure/element.pyx
and it is that method which is inheritated by coordinate functions.
comment:7 Changed 22 months ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
Ah, right, it is a subclass of Element
. Thanks.
comment:8 Changed 22 months ago by
Many thanks for the review!
comment:9 Changed 22 months ago by
 Branch changed from public/manifolds/coord_function_arithm22859 to 94e8c68536ffc0027c6f82177b10140dc43e58ef
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
Do not check for a zero result in arithmetics of coordinate functions and scalar fields