Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Python int problem with exp_integral #14766

Closed
kcrisman opened this issue Jun 18, 2013 · 10 comments
Closed

Fix Python int problem with exp_integral #14766

kcrisman opened this issue Jun 18, 2013 · 10 comments

Comments

@kcrisman
Copy link
Member

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

Depends on #17130

CC: @benjaminfjones @burcin @eviatarbach

Component: symbolics

Author: Jeroen Demeyer

Branch/Commit: 68c545a

Reviewer: Ralf Stephan

Issue created by migration from https://trac.sagemath.org/ticket/14766

@kcrisman kcrisman added this to the sage-5.11 milestone Jun 18, 2013
@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@eviatarbach
Copy link

comment:3

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.

@benjaminfjones
Copy link
Contributor

comment:4

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.

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@jdemeyer
Copy link

Branch: u/jdemeyer/ticket/14766

@jdemeyer
Copy link

Author: Jeroen Demeyer

@jdemeyer
Copy link

Commit: 68c545a

@jdemeyer
Copy link

Dependencies: #17130

@jdemeyer
Copy link

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

@rwst
Copy link

rwst commented Dec 12, 2014

Reviewer: Ralf Stephan

@rwst
Copy link

rwst commented Dec 12, 2014

comment:10

Looks fine and passes tests in functions/.

@vbraun
Copy link
Member

vbraun commented Dec 15, 2014

Changed branch from u/jdemeyer/ticket/14766 to 68c545a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants