behaviour of gamma strangely sensitive

[sagetrac-dsm]

Whether you get the correct result or a spurious ValueError in evaluating some gamma expressions depends upon things it shouldn't.

This comes from a sage-support thread; I looked and couldn't find it here, but possibly it's a duplicate. (I have bad luck with trac.)
See also this worksheet.

Here's a relatively simple test case, in 4.6.1 and 4.6.2.rc0:

sage: x = var("x")
sage: f = exp(gamma(I*x-1/2).subs(x=I*x))
sage: g=f.operands()[0]
sage: g, type(g)
(gamma(-x - 1/2), <type 'sage.symbolic.expression.Expression'>)
sage: g(x=0)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)

/Applications/sage_devel/<ipython console> in <module>()

/Applications/sage/local/lib/python2.6/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.__call__ (sage/symbolic/expression.cpp:15657)()

/Applications/sage/local/lib/python2.6/site-packages/sage/symbolic/ring.so in sage.symbolic.ring.SymbolicRing._call_element_ (sage/symbolic/ring.cpp:6537)()

/Applications/sage/local/lib/python2.6/site-packages/sage/symbolic/expression.so in sage.symbolic.expression.Expression.substitute (sage/symbolic/expression.cpp:15036)()

/Applications/sage/local/lib/python2.6/site-packages/sage/symbolic/pynac.so in sage.symbolic.pynac.py_doublefactorial (sage/symbolic/pynac.cpp:9463)()

ValueError: argument must be >= -1
sage: g(x=0)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
[ ... duplicate removed; this is merely to show it's not only the first call which has problems ]
ValueError: argument must be >= -1
sage: gamma(-x-1/2)*g
gamma(-x - 1/2)^2
sage: g(x=0)
-2*sqrt(pi)

That is, merely performing the should-be-irrelevant "gamma(-x-1/2)*g" op makes g(x=0) start working. Instrumenting it shows that that py_doublefactorial gets a -3 the first time instead of the 1 it does the second time, but I couldn't quite follow the calling order to isolate the error.

Apply attachment: trac_10849-doctests-on_9880.patch

Depends on #7160
Depends on #9880

CC: @kcrisman

Component: symbolics

Author: Burcin Erocal

Branch/Commit: 79a0ebd

Reviewer: Ralf Stephan

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

[mwhansen]

comment:1

I'm pretty sure this issue stems from comparison of number field elements. See #6132, #7160, #10064, and http://groups.google.com/group/sage-support/browse_thread/thread/28bbd04a78dadb57/01168722573ff736

[sagetrac-dsm]

comment:2

What weirds me out is the side-effect aspect: I could understand getting the same wrong answer, or different answers for equivalent but non-identical inputs, or even random behaviour. I don't see why the gamma(-x-1/2)*g call should change the state so that any element comparison would behave differently, but I understand exactly nothing about Sage internals at this level.

[burcin]

comment:3

Mike is right and the patch at #10064 resolves this issue.

The side effect is a result of GiNaC's reference counted pointers. Whenever two expressions compare equal, GiNaC frees the memory of one, and makes it a pointer to the other. In this example, it replaces -x - 1/2 where the coefficient of x is in QQ(i), with -x - 1/2 where the coefficient is just an int.

sage: t = -x -1/2
sage: t
-x - 1/2
sage: t.operands()
[-x, -1/2]
sage: t.operands()[0]
-x
sage: t.operands()[0].operands()
[x, -1]
sage: t.operands()[0].operands()[1]
-1
sage: t.operands()[0].operands()[1].pyobject()
-1
sage: type(t.operands()[0].operands()[1].pyobject())
<type 'int'>

Of course there is a chance that this observation changed the result. :)

For g, this looks like a different bug:

sage: g
gamma(-x - 1/2)
sage: g.operands()[0]
-x - 1/2
sage: g.operands()[0].operands()
[x, -1/2]
sage: g.operands()[0].operands()[0]
x
sage: g.operands()[0].operands()[0].operands()
[]

[burcin]

Author: Burcin Erocal

[burcin]

Dependencies: #7160

[burcin]

comment:5

This is fixed with the patch attached to #7160. Doctests are in attachment: trac_10849-doctests.patch.

[tkluck]

comment:6

I'll set this one to needs work because it depends on a ticket (#7160) that needs work, and it can't be reviewed before that one.

[kcrisman]

comment:7

One can also give it positive review and set it to sage-pending like at #10064. That's a little more informative, anyway.

[tkluck]

comment:8

We'll still have to check whether the patch still applies cleanly when the other ticket has been merged (at which point a lot of unrelated changes may have made their way into Sage too). That would keep me from giving it the sort of preliminary positive review you suggest.

But if what you're saying is the standard way of handling these things, then I'll raise no objection.

[kcrisman]

comment:9

Replying to @tkluck:

We'll still have to check whether the patch still applies cleanly when the other ticket has been merged (at which point a lot of unrelated changes may have made their way into Sage too). That would keep me from giving it the sort of preliminary positive review you suggest.

Of course you're right! But at least this would mean that modulo those other changes, the patch does what it says. Otherwise we might lose the info of a positive review.

How about I set it to sage-pending, and you set it to positive review if you feel that way? :-)

[burcin]

Attachment: trac_10849-doctests-on_9880.patch.gz

[burcin]

comment:10

Now that #7160 is fixed, this can be reviewed. Patchbot says that the old patch applies without problems on Sage 5.10.beta4 and passes tests. I am attaching a new patch that is rebased on #9880, since both tickets add doctests to the same function and I hope #9880 will go in before this one. :)

[burcin]

Changed dependencies from #7160 to #7160, #9880

[rwst]

Branch: u/rws/ticket/10849

[rwst]

comment:14

This is just a doctest which passes.

---

New commits:

dfaa889

trac 10849: add doctests for fix to number field element comparison

[rwst]

Commit: dfaa889

[rwst]

Reviewer: Ralf Stephan

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

79a0ebd

Merge branch 'develop' into ticket/10849

[sagetrac-git]

Changed commit from dfaa889 to 79a0ebd

[vbraun]

Changed branch from u/rws/ticket/10849 to 79a0ebd