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

WIP: Use single call to dsfmt_gv_genrand_uint32() for small ranges #5578

Closed
wants to merge 8 commits into from

Conversation

mschauer
Copy link
Contributor

This is just an idea regarding #5549, this way one looses the type stability in RandIntGen, but gains faster number generation, when length(r) <= typemax(Uint32) and only one call to dsfmt_gv_genrand_uint32 to obtain entropy is needed.

@JeffBezanson
Copy link
Sponsor Member

This is probably not going to be a good place to give up type stability.

@mschauer
Copy link
Contributor Author

I could not see any time difference in repeated calls to rand(int64(1:10)), rand(int64(1:10), 10000) on the other hand is two times faster than before here. But still worth to make everything type stable if this idea is interesting at all.

@mschauer
Copy link
Contributor Author

I have changed the procedure a bit and rebased. The overall performance of seperate calls is slightly better than before due to less calls to rand(Uint32) for small ranges:

Master:

julia> @timen 1000000 rand(uint(1:10^15));
elapsed time: 0.071746714 seconds (0 bytes allocated)

julia> @timen 1000000 rand(uint(1:10^5));
elapsed time: 0.067608864 seconds (0 bytes allocated)

PR:

julia> @timen 1000000 rand(uint(1:10^15));
elapsed time: 0.07122678 seconds (0 bytes allocated)

julia> @timen 1000000 rand(uint(1:10^5));
elapsed time: 0.057847697 seconds (0 bytes allocated)

Note that these are hard to compare, because of the rejection part of the algorithm.


function rand{T<:Integer}(g::RandIntGen{T,Uint64})
local x::Uint64
if g.k >> 32 == 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The indentation of this function seems to be a little bit messy. Would you please clean up following julia's four-space convention?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, some hidden tabs removed.

@lindahua
Copy link
Contributor

What's the performance of rand(1:1000, 10^6) before & after?

@mschauer
Copy link
Contributor Author

I get

before> @time rand(1:1000, 10^6);
elapsed time: 0.038024456 seconds (8000160 bytes allocated)
after> @time rand(1:1000, 10^6);
elapsed time: 0.029658467 seconds (8000160 bytes allocated)

@lindahua
Copy link
Contributor

Good.

+1 for merging this.

@mschauer
Copy link
Contributor Author

I'll test on 32 bit.

@StefanKarpinski
Copy link
Sponsor Member

This is probably not going to be a good place to give up type stability.

You don't return a different type. The point is that you only need 32 bits of entropy to return an Int64 within the range of Int32. If the amount of entropy used by rand(1:n) only depends on n and not on the type of n then you can get the same sequence of random values on both 32- and 64-bit systems, even though the types of the values will be different. Being able to generate consistent sequences across platforms with different word sizes is important.

if g.k >> 32 == 0
x = convert(Uint64,rand(Uint32))
while x >= g.u
x = convert(Uint64,rand(Uint32))
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These convert calls are unnecessary since x is a typed variable.

@StefanKarpinski
Copy link
Sponsor Member

@mschauer, can you clarify about the type stability? Does this actually introduce any?

@mschauer
Copy link
Contributor Author

No, I was maybe too generous with rebasing, this does not apply anymore.

@JeffBezanson
Copy link
Sponsor Member

Only a very early version of this was type-unstable. No longer relevant. This seems like a good change.

@mschauer
Copy link
Contributor Author

I found a bug in the constructors and need a bit more time testing this. A problem is not straight forward to test because the results only can differ if rejections happen, which is unfrequent. I will add some real tests too.

@mschauer
Copy link
Contributor Author

I am puzzled how the new function be faster than the old one for big ranges...
it only happens for certain ranges, for example for 0x1:0x0000040000000000 (the red dot is below the black one right in the figure.)

After some trial I am thinking that either this happens because the closing modulo operation takes less time or or I am missing something crucial.

rand32
Timings for some different sized ranges of rand(1:t, 10^6). PR is red dots, master black circles. Timings really depend on the range size, as g.a + rem(x, g.k) takes different time in each case. Red and black lines are PR and master without the division operation (replaced by g.a + x).

PS: The small peak at 0x1:0x0000000080000000 is expected, as this is the range which just doesn't fit two times into typemax(Uint32).

mschauer referenced this pull request Jan 31, 2014
…y reflectors. Add Float16 and BigFloat to tests and test promotion. Cleaup promotion in LinAlg. Avoid promotion in mutating ! functions. Make Symmetric, Hermitian and QRs immutable. Make thin a keyword argument in svdfact. Remove cond argument in sqrtm.
@mschauer
Copy link
Contributor Author

mschauer commented Feb 2, 2014

To test I replaced the random number generator with a pseudo"random" number generator that just counts 0x00000000, 0x00000001, etc and everything is fine. Once #5585 converges to something things will likely change and #5550 can be fixed, should one wait or do this in a separate pull request?

@mschauer
Copy link
Contributor Author

mschauer commented Feb 9, 2014

I will take 5585 into account and submit a new pull request.
@JeffBezanson : On jb/range1change I noted

int32(2:3)
> 2:4

@mschauer
Copy link
Contributor Author

mschauer commented Feb 9, 2014

Closed in favor of 5741

@mschauer mschauer closed this Feb 9, 2014
@ViralBShah ViralBShah added the randomness Random number generation and the Random stdlib label Nov 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
randomness Random number generation and the Random stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants