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

itrunc boundary issues #7517

Closed
simonbyrne opened this issue Jul 4, 2014 · 7 comments
Closed

itrunc boundary issues #7517

simonbyrne opened this issue Jul 4, 2014 · 7 comments
Labels
won't change Indicates that work won't continue on an issue or pull request

Comments

@simonbyrne
Copy link
Contributor

Ah the joys of edge cases...

julia> x = typemax(Int64)
9223372036854775807

julia> y = float64(x)
9.223372036854776e18

julia> itrunc(y)
-9223372036854775808

convert(Int64,y) and the other rounding functions (iceil, ifloor, iround) all throw InexactError().

@timholy
Copy link
Member

timholy commented Jul 4, 2014

Hmm. One problem with fixing this is that currently you can use itrunc to build your own much faster variant of ifloor. If we fix this, that will no longer be an option. I personally would like to have the option of a fast-but-dangerous version of ifloor---there are too many computations where ifloor is the bottleneck. As some justification, to me itrunc means "do it just like in C" (i.e., machine arithmetic).

@simonbyrne
Copy link
Contributor Author

@timholy What makes it faster? Is it just the avoidance of a comparison branch?

Actually it turns out that this isn't machine arithmetic, as the same thing happens for any value outside the range, e.g.

julia> itrunc(nextfloat(y))
-9223372036854775808

According to the C11 spec, "If the value of the integral part cannot be represented by the integer type, the behavior is undefined"

@JeffBezanson
Copy link
Member

Yes our itrunc is just not checked. In the future we'll probably also require this function for unchecked truncation of integers, so it's likely to stay this way.

@StefanKarpinski
Copy link
Member

Should we have this be safe by default and have an unsafe version for people who need it?

@JeffBezanson
Copy link
Member

I think in 0.3, for better or worse, this will be the unsafe version. We can change this when we start checking integer conversions.

@StefanKarpinski
Copy link
Member

Seems like a good plan. Do we have an issue for that where we can add this as a todo item?

@timholy
Copy link
Member

timholy commented Jul 5, 2014

@simonbyrne, yes, branches are very expensive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
won't change Indicates that work won't continue on an issue or pull request
Projects
None yet
Development

No branches or pull requests

4 participants