Opened 20 months ago

Last modified 5 weeks 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:

Status badges

Description

sage: 2^(-21111111111/11111111111)
...
RuntimeError: 

Why is Sage unable to evaluate this expression?

Change History (20)

comment:1 Changed 17 months ago by mkoeppe

  • Milestone changed from sage-9.1 to sage-9.2

Moving tickets to milestone sage-9.2 based on a review of last modification date, branch status, and severity.

comment:2 Changed 13 months ago by mkoeppe

  • Priority changed from major to critical

comment:3 Changed 12 months ago by gh-bmlivin

  • Owner changed from (none) to gh-bmlivin

comment:4 Changed 12 months ago by gh-bmlivin

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.

Last edited 12 months ago by gh-bmlivin (previous) (diff)

comment:5 Changed 12 months ago by gh-bmlivin

  • 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 12 months ago by gh-bmlivin

  • 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 11 months ago by mkoeppe

  • Milestone changed from sage-9.2 to sage-9.3

comment:8 Changed 11 months ago by gh-bmlivin

  • 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 8 months ago by gh-bmlivin

This should be fixed by the new commits to #30446. Since they're not merged in yet, I'm not sure whether to change this to needs review or not.

Last edited 8 months ago by gh-bmlivin (previous) (diff)

comment:10 Changed 8 months ago by gh-bmlivin

  • Dependencies set to #30446, #31118

comment:11 Changed 8 months ago by gh-bmlivin

  • Authors set to Ben Livingston

comment:12 Changed 8 months ago by zimmerma

thanks let's look again once #30446 is merged

comment:13 Changed 7 months ago by gh-DaveWitteMorris

  • Branch set to public/29156

comment:14 Changed 7 months ago by gh-DaveWitteMorris

  • Authors changed from Ben Livingston to Ben Livingston, Dave Morris
  • Commit set to 09e62e38c0b15df765c435542fe5a7076830052f
  • Status changed from new to needs_review

This no longer gives an error, because it was fixed by the pynac update in #30446, as mentioned above. The PR just adds a doctest.


New commits:

09e62e3trac 29156 doctest

comment:15 Changed 7 months ago by klee

  • Reviewers set to Kwankyu Lee
  • Status changed from needs_review to positive_review

comment:16 Changed 7 months ago by klee

Looks good.

comment:17 Changed 7 months ago by gh-DaveWitteMorris

Thanks!

comment:18 Changed 6 months ago by vbraun

  • 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 5 months ago by mkoeppe

  • Milestone changed from sage-9.3 to sage-9.4

Moving to 9.4, as 9.3 has been released.

comment:20 Changed 5 weeks ago by mkoeppe

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