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: |
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
- Branch set to public/31986
comment:2 Changed 14 months ago by
- Commit set to 5e08324b51b17a2aa1a97cea4050fa6d057eb5ff
comment:3 Changed 8 months ago by
- Milestone changed from sage-9.5 to sage-9.6
comment:4 Changed 4 months ago by
- Milestone changed from sage-9.6 to sage-9.7
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:
trac 31986 allow long values in pynac