sagemath / sage

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

trivial __copy__ and __deepcopy__ methods for symbolic expressions and functions #32480

Open mkoeppe opened 3 years ago

mkoeppe commented 3 years ago

By definition, symbolic expressions are immutable (but see #32450).

We define trivial __copy__ and __deepcopy__ methods that just return self.

Before:

sage: ex = sin(x)                                                                                                                                              
sage: %timeit copy(sin)                                                                                                                                        
7.06 µs ± 91.8 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
sage: %timeit deepcopy(sin)                                                                                                                                    
7.91 µs ± 511 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
sage: %timeit copy(ex)                                                                                                                                         
339 ns ± 2.64 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
sage: %timeit deepcopy(ex)                                                                                                                                     
29.7 µs ± 259 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)

After:

sage: %timeit copy(sin)                                                                                                                                        
263 ns ± 1.57 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
sage: %timeit deepcopy(sin)                                                                                                                                    
440 ns ± 3.93 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
sage: %timeit copy(ex)                                                                                                                                   
257 ns ± 2.17 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
sage: %timeit deepcopy(ex)                                                                                                                                     
429 ns ± 3.69 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

Also fixes #30018

CC: @egourgoulhon @tscrim

Component: symbolics

Author: Matthias Koeppe

Branch/Commit: u/mkoeppe/trivialcopyanddeepcopymethods_for_symbolic_expressions_and_functions @ f828986

Reviewer: Travis Scrimshaw

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

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -12,4 +12,11 @@
 29.7 µs ± 259 ns per loop (mean ± std. dev. of 7 runs, 10000 loops each)

+After:

+ +sage: %timeit copy(ex) +257 ns ± 2.17 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) +sage: %timeit deepcopy(ex) +429 ns ± 3.69 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) +

mkoeppe commented 3 years ago

Author: Matthias Koeppe

mkoeppe commented 3 years ago

Branch: u/mkoeppe/trivialcopyanddeepcopymethods_for_symbolic_expressions_and_functions

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

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

f828986Function.__copy__, __deepcopy__: Immutable, so just return self
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Commit: f828986

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -6,6 +6,10 @@

sage: ex = sin(x)
+sage: %timeit copy(sin)
+7.06 µs ± 91.8 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each) +sage: %timeit deepcopy(sin)
+7.91 µs ± 511 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each) sage: %timeit copy(ex)
339 ns ± 2.64 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each) sage: %timeit deepcopy(ex)
@@ -15,8 +19,13 @@ After:

+sage: %timeit copy(sin)                                                                                                                                        
+263 ns ± 1.57 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
+sage: %timeit deepcopy(sin)                                                                                                                                    
+440 ns ± 3.93 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
 sage: %timeit copy(ex)                                                                                                                                   
 257 ns ± 2.17 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)
 sage: %timeit deepcopy(ex)                                                                                                                                     
 429 ns ± 3.69 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

+

mkoeppe commented 3 years ago

Description changed:

--- 
+++ 
@@ -29,3 +29,4 @@
 429 ns ± 3.69 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

+Also fixes #30018

tscrim commented 3 years ago
comment:6

Green bot => positive review.

tscrim commented 3 years ago

Reviewer: Travis Scrimshaw

mkoeppe commented 3 years ago
comment:7

A related question here is whether we want to make Expression hashable even if the wrapped Python object is not, for example by falling back to taking the hash of the repr. Example:

sage: s = SR(matrix(QQ, [[1, 2], [3, 4]]))                                                                                                                     
sage: hash(s)                                                                                                                                                  
---------------------------------------------------------------------------
RuntimeError                              Traceback (most recent call last)
<ipython-input-2-9333020f3184> in <module>
----> 1 hash(s)

~/s/sage/sage-rebasing/worktree-gcc11/src/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression.__hash__()
   1947         sig_on()
   1948         try:
-> 1949             return self._gobj.gethash()
   1950         finally:
   1951             sig_off()

RuntimeError: Python object not hashable