Issue when pickling a formal function

[sagetrac-cdsousa]

There is a problem in symbolic function pickling:

sage: x = var('x'); f = function('f',x) ; s = dumps(f) ; loads(s)
RuntimeError: unknown function 'f' in archive

The error was not present in Sage 4.7 but it is in newer versions.
It was suggested (http://groups.google.com/group/sage-support/browse_thread/thread/b439844f2fa0b675) that it is related to Pynac.
Pynac was updated to 0.2.2 (sage-4.7.1.alpha1) and then to 0.2.3
(sage-4.7.1.alpha4) according to sage 4.7.1 changelog.

CC: @jpflori @burcin @jasongrout @sagetrac-dsm @orlitzky

Component: symbolics

Keywords: pynac

Author: Michael Orlitzky

Reviewer: Nils Bruin, Burcin Erocal

Merged: sage-5.1.beta1

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

[nbruin]

Changed keywords from none to pynac

[orlitzky]

comment:5

I hit this independently using deepcopy(). In sage-4.7:

sage: f = function('f',x)
sage: deepcopy(f)
f(x)

and in 4.8.alpha5:

sage: f = function('f',x)
sage: deepcopy(f)
...
RuntimeError: unknown function 'f' in archive

[orlitzky]

comment:6

Interestingly, if I take the pynac spkg from 4.7 and install it on 5.0.beta7, it doesn't fix the crash.

[orlitzky]

comment:7

I built a copy of 4.7.1, and the crash does happen. So the change was introduced between 4.7 and 4.7.1.

[orlitzky]

comment:8

Okay, this broke with #9240. I can fix it with a one-line change in 4.7.1.alpha4:

diff --git a/sage/symbolic/function.pyx b/sage/symbolic/function.pyx
--- a/sage/symbolic/function.pyx
+++ b/sage/symbolic/function.pyx
@@ -127,6 +127,7 @@
     cdef _register_function(self):
         cdef GFunctionOpt opt
         opt = g_function_options_args(self._name, self._nargs)
+        opt.set_python_func()

         if hasattr(self, '_eval_'):
             opt.eval_func(self)

But there's no longer a set_python_func() method on GFunctionOpt, so I don't know how to fix it in 5.0.beta7. The adventure continues...

[orlitzky]

comment:9

Replying to @orlitzky:

But there's no longer a set_python_func() method on GFunctionOpt, so I don't know how to fix it in 5.0.beta7. The adventure continues...

Disregard that, I'm a moron. Testing a patch now.

[orlitzky]

Attachment: sage-trac_11919.patch.gz

[orlitzky]

Author: Michael Orlitzky

[sagetrac-abid-naqvi83]

comment:11

I had the same problem on Sage 4.7.1. I applied the patch described above but it didn't work (same error message). I then uninstalled sage and installed version 4.8 instead. I applied the patch again and it still didn't work. Any suggestions?

[orlitzky]

comment:12

Replying to @sagetrac-abid-naqvi83:

I had the same problem on Sage 4.7.1. I applied the patch described above but it didn't work (same error message). I then uninstalled sage and installed version 4.8 instead. I applied the patch again and it still didn't work. Any suggestions?

Did you remember to run sage -b after applying the patch?

I don't remember which version I created the patch against... Does it at least apply cleanly against 4.8?

[nbruin]

comment:13

Fix confirmed! indeed, the patch simply adds back a line that disappeared in #9240 without any reason stated, so probably that was just an accident.

Positive review!

(for abid_naqvi83: note that the patchbot is happy and that the feature is actually doctested. If the fix has no effect, perhaps you forgot to run "sage -b"?)

[sagetrac-abid-naqvi83]

comment:14

That was it. Didn't know about "sage -b". Sorry for the faux pas and thanks for the fix. Working now.

[burcin]

comment:15

Replying to @nbruin:

Fix confirmed! indeed, the patch simply adds back a line that disappeared in #9240 without any reason stated, so probably that was just an accident.

It was not an accident. The python_func flag stored in pynac is not a bool any more. It is a bitmask that marks which custom functions are implemented in Python. The rest, if they exist are C++ functions.

Positive review!

This patch doesn't fix the problem. It might actually lead to crashes, since pynac will look for a python function to call for evaluation, differentiation, etc. if some bits in python_func are set.

The correct fix will be to get pynac to create a new dummy symbolic function at the point where it raises an error with "unknown function 'f' in archive." Relevant code can be found in ginac/function.{h,cpp} in pynac sources.

[burcin]

comment:16

I take it back. This is a good workaround, since constructors of function_options takes care to clear the function pointers used to store references to the custom methods which can be defined for evalutation, conjugation, etc. Before trying to evaluate any of these custom methods, we check if the pointer is NULL, so a crash can never happen.

[burcin]

Reviewer: Nils Bruin, Burcin Erocal

[burcin]

comment:18

Pynac 0.2.4 from #12950 fixes this, without the patch attached to this ticket. I don't know how long it will take to get that reviewed and merged, so I'm not opposed to merging this first.

[jdemeyer]

Merged: sage-5.1.beta1

[orlitzky]

comment:20

Replying to @burcin:

Pynac 0.2.4 from #12950 fixes this, without the patch attached to this ticket. I don't know how long it will take to get that reviewed and merged, so I'm not opposed to merging this first.

Now that #12950 is in, should we remove the line again (but keep the test)?

[burcin]

comment:21

Replying to @orlitzky:

Replying to @burcin:

Pynac 0.2.4 from #12950 fixes this, without the patch attached to this ticket. I don't know how long it will take to get that reviewed and merged, so I'm not opposed to merging this first.

Now that #12950 is in, should we remove the line again (but keep the test)?

Yes, though it is mostly harmless AFAICT.

[orlitzky]

comment:22

I've removed it at #13446 (needs review if anyone wants it).