Opened 12 years ago

Closed 11 years ago

#10736 closed defect (fixed)

int raised to a RealNumber gives an error

Reported by: Jason Grout Owned by: Alex Ghitza
Priority: major Milestone: sage-4.7.2
Component: basic arithmetic Keywords:
Cc: Dan Drake Merged in: sage-4.7.2.alpha0
Authors: Robert Bradshaw Reviewers: Dan Drake, Mariah Lenox
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Mariah Lennox)

Dan Drake reported this error:

sage: int(1.5)**(2.1)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)

/Users/grout/Documents/<ipython console> in <module>()

TypeError: unsupported operand type(s) for ** or pow(): 'int' and 'sage.rings.real_mpfr.RealLiteral'

Apply: 10736-real-pow-v2.patch

Attachments (2)

10736-real-pow.patch (2.8 KB) - added by Robert Bradshaw 12 years ago.
10736-real-pow-v2.patch (3.0 KB) - added by Dan Drake 11 years ago.
replaces previous

Download all attachments as: .zip

Change History (15)

comment:1 Changed 12 years ago by Dan Drake

Note that this works properly for rationals:

sage: int(2)**(3/2)
2*sqrt(2)
sage: parent(_)
Symbolic Ring

Also, this thing is likely to happen in list comprehensions using range(), since that returns Python ints. Right now, this doesn't work as expected:

[n^(1.5) for n in range(10)]

comment:2 Changed 12 years ago by Robert Bradshaw

Status: newneeds_review

Changed 12 years ago by Robert Bradshaw

Attachment: 10736-real-pow.patch added

comment:3 Changed 12 years ago by Dan Drake

This passes all doctests and -- with my minimal knowledge of Cython and MPFR -- the code looks very good. I see that the relevant function automatically coerces to complex numbers; would it make sense to add a doctest for negative ints? And maybe zero, to be complete?

sage: int(-2)^(0.333333)
0.629961522017056 + 1.09112272417509*I
sage: int(0)^(1.0)
0.000000000000000
sage: int(0)^(0.0)
1.00000000000000

My guess is that we'll need to test this on a bunch of platforms to suss out any numerical noise. But other than that, I can give this a weak positive review -- we need someone who knows MPFR and Cython better than I do. (Or, I need to learn more of those things...)

comment:4 Changed 12 years ago by Robert Bradshaw

MPFR is platform independent--it's based on gmp/mpir so there's no "numerical noise" issues to worry about as there's no (machine-level) floating point arithmetic.

comment:5 Changed 11 years ago by Karl-Dieter Crisman

My knowledge of Cython and MPFR is even more minimal. But the word "track" is in this ticket, should probably be fixed.

Changed 11 years ago by Dan Drake

Attachment: 10736-real-pow-v2.patch added

replaces previous

comment:6 Changed 11 years ago by Dan Drake

Description: modified (diff)

comment:7 Changed 11 years ago by Dan Drake

I fixed the "track #10736" bit and added the doctests I mentioned above to the "v2" patch.

comment:8 Changed 11 years ago by Karl-Dieter Crisman

Authors: Robert Bradshaw
Reviewers: Dan Drake

comment:9 Changed 11 years ago by Karl-Dieter Crisman

Dan, anything you think still needs to be reviewed here? That would be helpful for any future reviewers - sounds like you want a Cython guru to check it, correct, otherwise all is well?

comment:10 in reply to:  9 Changed 11 years ago by Dan Drake

Replying to kcrisman:

Dan, anything you think still needs to be reviewed here? That would be helpful for any future reviewers - sounds like you want a Cython guru to check it, correct, otherwise all is well?

Yep, just a quick once-over by someone who knows Cython and maybe a bit about MPFR. The code is simple and I can see it's very reasonable (and it fixes the problem!), but I'd like another person to look at it.

comment:11 Changed 11 years ago by Mariah Lennox

Description: modified (diff)
Reviewers: Dan DrakeDan Drake, Mariah Lenox
Status: needs_reviewpositive_review

Applied patch to sage-4.7.1.alpha2, did 'sage -b' then 'make testlong'. The patch fixed the reported problem. All tests passed. Positive review!

comment:12 Changed 11 years ago by Jeroen Demeyer

Milestone: sage-4.7.1sage-4.7.2

comment:13 Changed 11 years ago by Jeroen Demeyer

Merged in: sage-4.7.2.alpha0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.