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

Better fix for #8871 #8893

Merged
merged 1 commit into from
Nov 4, 2014
Merged

Better fix for #8871 #8893

merged 1 commit into from
Nov 4, 2014

Conversation

ivarne
Copy link
Sponsor Member

@ivarne ivarne commented Nov 3, 2014

Mapreduce didn't work with Range{BigInt}, because the length wasn't a
Int, because of potential overflow. This patch just converts the
length to Int with a checked conversion, because a mapreduce on larger
arrays will probably never finish anyway.

Also fix some broken test of prod

Mapreduce didn't work with `Range{BigInt}`, because the length wasn't a
`Int`, because of potential overflow. This patch just converts the
length to Int with a checked conversion, because a mapreduce on larger
arrays will probably never finish anyway.

Also fix some broken test of prod
@ivarne
Copy link
Sponsor Member Author

ivarne commented Nov 3, 2014

@LMSK What do you think about this for #8871?

@ghost
Copy link

ghost commented Nov 3, 2014

Works for me, thanks.

@ivarne
Copy link
Sponsor Member Author

ivarne commented Nov 4, 2014

@lindahua You wrote most of this part of Julia. Can you have a quick look at this, and merge if it looks good?

@lindahua
Copy link
Contributor

lindahua commented Nov 4, 2014

@ivarne It looks good to me. The travis failure seems to be irrelevant. Ok, I will go ahead to merge this.

lindahua added a commit that referenced this pull request Nov 4, 2014
@lindahua lindahua merged commit b7feae3 into master Nov 4, 2014
@ivarne
Copy link
Sponsor Member Author

ivarne commented Nov 4, 2014

Thanks!

cc: @JuliaBackports (As this fixes a bug and makes other things that never worked an error.)

ivarne added a commit that referenced this pull request Nov 4, 2014
The test added in #8893 shouldn't use typemax(Int), but typemax(Int64)
@ivarne ivarne deleted the bigint_prod branch November 7, 2014 10:23
ivarne added a commit that referenced this pull request Nov 7, 2014
Mapreduce didn't work with `Range{BigInt}`, because the length wasn't a
`Int`, because of potential overflow. This patch just converts the
length to Int with a checked conversion, because a mapreduce on larger
arrays will probably never finish anyway.

Also fix some broken test of prod

(cherry picked from commit 9493465)

Fix test for 32 bit

The test added in #8893 shouldn't use typemax(Int), but typemax(Int64)

(cherry picked from commit 49083be)
@ivarne
Copy link
Sponsor Member Author

ivarne commented Nov 7, 2014

Backported both fix, and test fix in 245b9f6

waTeim pushed a commit to waTeim/julia that referenced this pull request Nov 23, 2014
The test added in JuliaLang#8893 shouldn't use typemax(Int), but typemax(Int64)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants