Opened 21 months ago
Last modified 2 months ago
#29156 needs_work defect
runtime error with simple expression
Reported by: | zimmerma | Owned by: | gh-bmlivin |
---|---|---|---|
Priority: | critical | Milestone: | sage-9.5 |
Component: | basic arithmetic | Keywords: | |
Cc: | Merged in: | ||
Authors: | Ben Livingston, Dave Morris | Reviewers: | Kwankyu Lee |
Report Upstream: | Fixed upstream, in a later stable release. | Work issues: | |
Branch: | public/29156 (Commits, GitHub, GitLab) | Commit: | 09e62e38c0b15df765c435542fe5a7076830052f |
Dependencies: | #30446, #31118 | Stopgaps: |
Description
sage: 2^(-21111111111/11111111111) ... RuntimeError:
Why is Sage unable to evaluate this expression?
Change History (20)
comment:1 Changed 18 months ago by
- Milestone changed from sage-9.1 to sage-9.2
comment:2 Changed 14 months ago by
- Priority changed from major to critical
comment:3 Changed 14 months ago by
- Owner changed from (none) to gh-bmlivin
comment:4 Changed 13 months ago by
This is something that might be a bug in pynac, or it might be that Sage is using pynac incorrectly. Here is an excerpt from pynac's numeric::power
in numeric.cpp
, which is what ends up being called in this example:
if (exponent.is_negative()) { long int_exp = -(expo.to_long()); ... }
In your example, exponent
is what you think it should be (-21111111111/11111111111). Here's what to_long
does:
if (mpz_fits_sint_p(v._bigint)) { long n = mpz_get_si(bigint); mpz_clear(bigint); return n; } mpz_clear(bigint); throw conversion_error();
Here, v._bigint
is the absolute value of the exponent's numerator, 21111111111. That happens to be a bit larger than 32 bits, so the if statement doesn't execute, and throw_conversion_error
is called.
I haven't combed through pynac's documentation to see whether they point out that numeric::power
shouldn't be called with negative rational exponents whose numerators are greater than 32 bits, but I can't see why they would want to do this and not do the same with positive numerators or denominators greater than 32 bits. So I think this is a bug in pynac.
comment:5 Changed 13 months ago by
- Report Upstream changed from N/A to Not yet reported upstream; Will do shortly.
It may be a while before I can report this upstream. If someone else would like to do so, please go ahead.
comment:6 Changed 13 months ago by
- Report Upstream changed from Not yet reported upstream; Will do shortly. to Reported upstream. No feedback yet.
See https://github.com/pynac/pynac/issues/356 for the upstream report.
comment:7 Changed 12 months ago by
- Milestone changed from sage-9.2 to sage-9.3
comment:8 Changed 12 months ago by
- Report Upstream changed from Reported upstream. No feedback yet. to Fixed upstream, in a later stable release.
I've submitted a pull request for this with pynac
: https://github.com/pynac/pynac/pull/358. That said, my pull request does not fix the underlying issue of dealing with negative exponents. It only loosens the requirement from the numerator needing to fit in a C
int
to needing to fit in a C
long
. That fixes the specific example reported in this ticket, but if someone wanted to raise to a rational power with a numerator greater than 63 bits, it would still raise an error.
comment:9 Changed 9 months ago by
This should be fixed by the new commits to #29156. Since they're not merged in yet, I'm not sure whether to change this to needs review or not.
comment:10 Changed 9 months ago by
- Dependencies set to #30446, #31118
comment:11 Changed 9 months ago by
comment:12 Changed 9 months ago by
thanks let's look again once #30446 is merged
comment:13 Changed 8 months ago by
- Branch set to public/29156
comment:14 Changed 8 months ago by
- Commit set to 09e62e38c0b15df765c435542fe5a7076830052f
- Status changed from new to needs_review
comment:15 Changed 8 months ago by
- Reviewers set to Kwankyu Lee
- Status changed from needs_review to positive_review
comment:16 Changed 8 months ago by
Looks good.
comment:17 Changed 8 months ago by
Thanks!
comment:18 Changed 7 months ago by
- Status changed from positive_review to needs_work
On 32-bit:
********************************************************************** File "src/sage/rings/integer.pyx", line 2201, in sage.rings.integer.Integer.__pow__ Failed example: 2^(-21111111111/11111111111) Exception raised: Traceback (most recent call last): File "/home/buildbot/slave/sage_git/build/local/lib/python3.9/site-packages/sage/doctest/forker.py", line 714, in _run self.compile_and_execute(example, compiler, test.globs) File "/home/buildbot/slave/sage_git/build/local/lib/python3.9/site-packages/sage/doctest/forker.py", line 1133, in compile_and_execute exec(compiled, globs) File "<doctest sage.rings.integer.Integer.__pow__[22]>", line 1, in <module> Integer(2)**(-Integer(21111111111)/Integer(11111111111)) File "sage/rings/integer.pyx", line 2211, in sage.rings.integer.Integer.__pow__ (build/cythonized/sage/rings/integer.c:15193) return coercion_model.bin_op(left, right, operator.pow) File "sage/structure/coerce.pyx", line 1204, in sage.structure.coerce.CoercionModel.bin_op (build/cythonized/sage/structure/coerce.c:10671) return PyObject_CallObject(op, xy) File "sage/structure/element.pyx", line 2055, in sage.structure.element.Element.__pow__ (build/cythonized/sage/structure/element.c:14428) return (<Element>left)._pow_(right) File "sage/rings/rational.pyx", line 2602, in sage.rings.rational.Rational._pow_ (build/cythonized/sage/rings/rational.cpp:22251) return SR(c) * SR(d).power(n, hold=True) File "sage/structure/element.pyx", line 1513, in sage.structure.element.Element.__mul__ (build/cythonized/sage/structure/element.c:12170) return (<Element>left)._mul_(right) File "sage/symbolic/expression.pyx", line 3799, in sage.symbolic.expression.Expression._mul_ (build/cythonized/sage/symbolic/expression.cpp:25489) x = left._gobj * _right._gobj RuntimeError **********************************************************************
comment:19 Changed 6 months ago by
- Milestone changed from sage-9.3 to sage-9.4
Moving to 9.4, as 9.3 has been released.
comment:20 Changed 2 months ago by
- Milestone changed from sage-9.4 to sage-9.5
Moving tickets to milestone sage-9.2 based on a review of last modification date, branch status, and severity.