Issues pickling sympy.Function('f')

[asmeurer]

We have a test failure in our SymPy test suite from cloudpickle. I have bisected it to the cloudpickle commit aec80d2

The failure is described at sympy/sympy#15116

The basic failure is this:

>>> import cloudpickle
>>> from sympy import Function
>>> f = Function('f')
>>> f1 = cloudpickle.loads(cloudpickle.dumps(f))
>>> f.__dict__['_prop_handler']
{'commutative': <function Function._eval_is_commutative at 0x1121f27b8>, 'negative': <function Expr._eval_is_negative at 0x112112d08>, 'positive': <function Expr._eval_is_positive at 0x112112c80>, 'complex': <function Function._eval_is_complex at 0x1121f2840>}
>>> f1.__dict__['_prop_handler']
{'commutative': <function Function._eval_is_commutative at 0x1130a7d08>, 'negative': <function Expr._eval_is_negative at 0x1130a7e18>, 'positive': <function Expr._eval_is_positive at 0x11309bbf8>, 'complex': <function Function._eval_is_complex at 0x1130aae18>}
>>> from sympy import Expr
>>> Expr._eval_is_negative
<function Expr._eval_is_negative at 0x112112d08>

The SymPy test tests that all the attributes are the same, and fails because cloudpickle creates new instances of the Expr methods.

I'm not clear if this affects functionality or not.

CC @ssanderson

[ssanderson]

Interesting. It looks to me like what's happening here is that f has a dict that holds a bunch of bound methods of Function and Eval. If those are different as a result of aec80d2, the most likely culprit is that when we go to pickle those methods, we decide that we need to pickle copy the entire state of the class rather than just pickling as a module + attribute pair, which is the default behavior for classes. Are Function and Expr dynamically generated?

As a heads up, I'm travelling at JupyterCon this week, so I may not have time to look at this carefully until early next week some time.

[asmeurer]

No Expr is static. Only Function('f') is dynamic (which is just syntactic sugar that creates a dynamic subclass of Function called f). There are also several metaclasses involved.

The code is hereabouts.

There's no rush on this. I think we will just skip the test in SymPy, as it apparently hasn't worked in some time. I also don't really know if this affects actual behavior for the unpickled object, beyond memory leaking the recreated methods. Our pickling tests are rigorous and check every attribute, but as far as I can tell stuff like f == f1 and f(x) == f1(x) work just fine.

[pierreglaser]

Following up on this. Actually, f in your example is a class, as Function is a metaclass. As f was created in an interactive session, it is dynamic, and cloudpickle would generate instructions to reconstruct a new class from scratch at unpickling time, including new methods. Hence all the equality/identity assertion failures on f and its methods.

However, since #246, cloudpickle.loads became smarter when loading classes: if the class being loaded already exists in the current interpreter session, the loaded class is reconciled with it's pre-existing object.

So now I get:

In [1]: import cloudpickle 
   ...: import sympy 
   ...: import types 
   ...: f = sympy.Function('f') 
   ...: f1 = cloudpickle.loads(cloudpickle.dumps(f)) 
   ...: f1 is f                                                                                                                                                                                                    
Out[1]: True

@asmeurer you should now be able to unskip the test AFAICT.