Opened 10 years ago

Closed 8 years ago

#14766 closed defect (fixed)

Fix Python int problem with exp_integral

Reported by: kcrisman Owned by: burcin
Priority: major Milestone: sage-6.4
Component: symbolics Keywords:
Cc: benjaminfjones, burcin, eviatarbach Merged in:
Authors: Jeroen Demeyer Reviewers: Ralf Stephan
Report Upstream: N/A Work issues:
Branch: 68c545a (Commits, GitHub, GitLab) Commit: 68c545a5edb07ea87adea5850a143195723dfced
Dependencies: #17130 Stopgaps:

Status badges

Description

In #11143 we weren't careful about Python ints. There are lots of examples of this, so one will want to go through the whole file.

sage: exp_integral_e(int(3),0)
0
sage: exp_integral_e(3,0)
1/2

Change History (11)

comment:1 Changed 10 years ago by kcrisman

Type: PLEASE CHANGEdefect

comment:2 Changed 9 years ago by jdemeyer

Cc: benjaminfjones,burcin,benjaminfjones, burcin
Milestone: sage-5.11sage-5.12

comment:3 Changed 9 years ago by eviatarbach

Cc: eviatarbach added

I'm guessing this occurs in other files too.

I think it's unreasonable to expect everyone adding symbolic functions to worry about this, and certainly users to be aware of it; maybe we could make BuiltinFunction.__call__ automatically convert int to Integer? Or have it raise an error if an int is in the parameters? Allowing int into _eval_ causes other problems too, such as getting expressions with int pyobjects in them, making arithmetic slower, etc.

At the very least, we could temporarily add from __future__ import division in all files that implement symbolic functions; then exact answers wouldn't be returned in all cases where they should, but at least mathematically wrong answers wouldn't occur.

comment:4 Changed 9 years ago by benjaminfjones

I think my preference would be either raising a TypeError (and clearly documenting what the allowed types are) or doing Python 3 style division. I don't like the idea of adding an implicit coercion int -> Integer.

If we do type checking in BuiltinFunction.__call__, we should make sure that there isn't a significant performance penalty.

comment:5 Changed 9 years ago by vbraun_spam

Milestone: sage-6.1sage-6.2

comment:6 Changed 9 years ago by vbraun_spam

Milestone: sage-6.2sage-6.3

comment:7 Changed 8 years ago by vbraun_spam

Milestone: sage-6.3sage-6.4

comment:8 Changed 8 years ago by jdemeyer

Branch: u/jdemeyer/ticket/14766
Created: Jun 18, 2013, 5:37:43 PMJun 18, 2013, 5:37:43 PM
Modified: Aug 10, 2014, 4:51:03 PMAug 10, 2014, 4:51:03 PM

comment:9 Changed 8 years ago by jdemeyer

Authors: Jeroen Demeyer
Commit: 68c545a5edb07ea87adea5850a143195723dfced
Dependencies: #17130
Status: newneeds_review

New commits:

6d10729Simplify numerical evaluation of BuiltinFunctions
b6e1ed4Merge remote-tracking branches 'origin/u/jdemeyer/ticket/17131' and 'origin/u/jdemeyer/ticket/17133' into ticket/17130
382f97aMerge branch 'u/jdemeyer/ticket/17130' of trac.sagemath.org:sage into 6.5beta1
726598917130: reviewer's patch: fix typo
c47dbd4Fix Trac #17328 again in a better way
a486db2Call the factorial() method of an Integer
68c545aFix bugs due to Python 2 int division

comment:10 Changed 8 years ago by rws

Reviewers: Ralf Stephan
Status: needs_reviewpositive_review

Looks fine and passes tests in functions/.

comment:11 Changed 8 years ago by vbraun

Branch: u/jdemeyer/ticket/1476668c545a5edb07ea87adea5850a143195723dfced
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.