Opened 14 months ago

Last modified 4 months ago

#31986 new enhancement

Pynac should allow signed long variables to store large values

Reported by: gh-DaveWitteMorris Owned by:
Priority: major Milestone: sage-9.7
Component: symbolics Keywords: pynac, long
Cc: Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: public/31986 (Commits, GitHub, GitLab) Commit: 5e08324b51b17a2aa1a97cea4050fa6d057eb5ff
Dependencies: #31585, #31984 Stopgaps:

Status badges

Description

Pynac's numeric.cpp file includes the following function definition:

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);
        }
}

This means that if an integer value could fit into a C++ signed integer, then it will be stored as a signed long integer; otherwise, it will be stored as an mpz value. This is a waste of bits: a signed long integer can usually hold much larger values than a signed integer. Therefore, in order to use all of the bits of the signed long integer variable, this patch changes mpz_fits_sint_p to mpz_fits_slong_p.

This change may uncover bugs in the pynac code. For example, the bugs of #31585 were previously only seen on 32-bit, but they show up in 64-bit (after changing 2^31 to 2^63) if the patch here is applied without patches that solve the problems on that ticket:

sage: n = 2^63
sage: (2*x).subs(x=-n)
0
sage: abs(x).subs(x=-n)
-9223372036854775808
sage: (-x).subs(x=-n)
-9223372036854775808
sage: a,b,c,d = var("a b c d")
sage: ((a + b + c)^62 * (3*b + d - 5/d)^3).expand().subs(a=0,b=2,c=-1)
d^3 + 18*d^2 + 15439924789694894702685*d - 77199623948474473513425/d + 450/d^2 - 125/d^3 + 36
sage: (-n*x + 1)*x
------------------------------------------------------------------------
(no backtrace available)
------------------------------------------------------------------------
Unhandled SIGFPE: An unhandled floating point exception occurred.
This probably occurred because a *compiled* module has a bug
in it and is not properly wrapped with sig_on(), sig_off().
Python will now terminate.

It therefore seems that this ticket should be merged early in a release cycle, so I am suggesting 9.5 as the milestone.

Change History (4)

comment:1 Changed 14 months ago by gh-DaveWitteMorris

  • Branch set to public/31986

comment:2 Changed 14 months ago by gh-DaveWitteMorris

  • Commit set to 5e08324b51b17a2aa1a97cea4050fa6d057eb5ff

For now, if you want to try out the pynac patch on this ticket, you can merge the branch, and then ./sage -f pynac.


New commits:

5e08324trac 31986 allow long values in pynac

comment:3 Changed 8 months ago by mkoeppe

  • Milestone changed from sage-9.5 to sage-9.6

comment:4 Changed 4 months ago by mkoeppe

  • Milestone changed from sage-9.6 to sage-9.7
Note: See TracTickets for help on using tickets.