py3: pynac bug with logb on int

[embray]

On Python 2:

sage: from sage.functions.log import logb
sage: logb(7, 2)
log(7)/log2
sage: logb(int(7), 2)
log(7)/log(2)

On Python 3:

sage: from sage.functions.log import logb
sage: logb(7, 2)
log(7)/log(2)
sage: logb(int(7), 2)
1

This was causing quite a few confusing bugs before I realized the subtle difference of Integer vs int that was causing the bug. I'm not sure if it affect other functions too.

Upstream PR: ​https://github.com/pynac/pynac/pull/336 (for pynac)

Though we should also still fix conversion of integer types to expressions on Python 3.

[embray]

It only seems to be if the first argument is an int; the type of the base doesn't seem to matter.

[embray]

Not 100% sure if related, but an interesting difference on Python 2 and 3:

py2:

sage: SR.coerce(Integer(7))._dbgprint()
7 (numeric) @0x603ced1f0, hash=0x7, flags=0x7, type MPZ
sage: SR.coerce(int(7))._dbgprint()
7 (numeric) @0x6016ab100, hash=0x7, flags=0x7, type LONG

py3:

sage: SR.coerce(Integer(7))._dbgprint()
7 (numeric) @0x4ce0820, hash=0x7, flags=0x7, type LONG
sage: SR.coerce(int(7))._dbgprint()
7 (numeric) @0x4ce0820, hash=0x7, flags=0x7, type MPZ

I don't think these are the exact same pynac versions either, so it's not clear to me yet where that difference is coming from.

[embray]

Something like this seems to fix it, and makes more sense in terms of integer representation:

src/sage/symbolic/ring.pyx

@@ -14 +14 @@
 #*****************************************************************************
 from __future__ import absolute_import
 
+from sage.arith.long cimport integer_check_long_py
 from sage.ext.cplusplus cimport ccrepr
 
 from sage.libs.pynac.pynac cimport *
@@ -331 +332 @@
             TypeError: positive characteristic not allowed in symbolic computations
         """
         cdef GEx exp
+        cdef int err = 0
+        cdef long x_long
+
         if is_Expression(x):
             if (<Expression>x)._parent is self:
                 return x
@@ -342 +346 @@
             try:
                 from sage.calculus.calculus import symbolic_expression_from_string
                 return self(symbolic_expression_from_string(x))
-            except SyntaxError as err:
-                msg, s, pos = err.args
+            except SyntaxError as exc:
+                msg, s, pos = exc.args
                 raise TypeError("%s: %s !!! %s" %
                         (msg, bytes_to_str(s[:pos]), bytes_to_str(s[pos:])))
 
@@ -361 +365 @@
                 from sage.symbolic.constants import NaN
                 return NaN
             exp = x
-        elif isinstance(x, long):
-            exp = x
-        elif isinstance(x, int):
-            exp = GEx(<long>x)
+        elif integer_check_long_py(x, &x_long, &err):
+            if not err:
+                exp = GEx(x_long)
+            else:
+                exp = x
         elif x is infinity:
             return new_Expression_from_GEx(self, g_Infinity)
         elif x is minus_infinity:

[embray]

Despite the above fix, there is still an apparent problem:

sage: logb(2**30, 2)
30
sage: logb(2**31, 2)
1
sage: logb(2**32, 2)
1
sage: logb(2**33, 2)
1

...and so on.

[embray]

I'm still getting:

sage: SR.coerce(Integer(2**30))._dbgprint()
1073741824 (numeric) @0x3b29c60, hash=0x40000000, flags=0x7, type LONG
sage: SR.coerce(Integer(2**31))._dbgprint()
2147483648 (numeric) @0x3b29c60, hash=0x80000000, flags=0x7, type MPZ

This is originating from ginac/numeric.cpp, specifically:

void set_from(Type& t, Value& v, long& hash, mpz_t bigint)
{
        if (mpz_fits_sint_p(bigint)) {
                t = LONG;
                v._long = mpz_get_si(bigint);
                hash = (v._long==-1) ? -2 : v._long;
        }
        else {
                t = MPZ;
                mpz_init_set(v._bigint, bigint);
                hash = _mpz_pythonhash(v._bigint);
        }
}

One, perhaps naïve, question I have is why mpz_fits_sint_p and not mpz_fits_slong_p?

[embray]

So there's still a discrepancy between creating a pynac numeric from an Integer versus an int depending on what the value is. With my patch above, it will create a LONG type numeric up to int(2**63 - 1), or up to Integer(2**31 - 1).

But regardless, the logb function is broken for MPZ numerics.

[embray]

I found it; small two character bug in pynac (and maybe upstream in ginac?):

ginac/numeric.cpp

@@ -3758 +3758 @@
                 return c;
         }
         if (b.t == LONG)
-                return ratlog(to_bigint(), israt);
+                return ratlog(b.to_bigint(), israt);
         if (t == LONG)
                 return to_bigint().ratlog(b, israt);
         if (t == MPZ and b.t == MPZ) {

[chapoton]

@rws, please make a pynac release

[embray]

Retargeting some of my tickets (somewhat optimistically for now).

[chapoton]

see pynac update ticket in #26995

[chapoton]

one still needs to add a doctest ?

[chapoton]

doctest added

---

New commits:

​f96cd53

add one doctest for logb

[tscrim]

LGTM.