Skip to content

Commit

Permalink
small fixup for rand(::BigInt) and update docs
Browse files Browse the repository at this point in the history
Now the runtime version of GMP is checked against the compile time
version, as suggested by @JeffBezanson. He also noted that the limbs
field of RangeGeneratorBigInt was not specific enough.
  • Loading branch information
rfourquet committed Dec 3, 2014
1 parent 1673768 commit 42fd6b1
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 5 deletions.
12 changes: 10 additions & 2 deletions base/gmp.jl
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@ else
end
typealias CdoubleMax Union(Float16, Float32, Float64)

const GMP_VERSION = VersionNumber(bytestring(unsafe_load(cglobal((:__gmp_version, :libgmp), Ptr{Cchar}))))
const GMP_BITS_PER_LIMB = Int(unsafe_load(cglobal((:__gmp_bits_per_limb, :libgmp), Cint)))
gmp_version() = VersionNumber(bytestring(unsafe_load(cglobal((:__gmp_version, :libgmp), Ptr{Cchar}))))
gmp_bits_per_limb() = Int(unsafe_load(cglobal((:__gmp_bits_per_limb, :libgmp), Cint)))

const GMP_VERSION = gmp_version()
const GMP_BITS_PER_LIMB = gmp_bits_per_limb()

# GMP's mp_limb_t is by default a typedef of `unsigned long`, but can also be configured to be either
# `unsigned int` or `unsigned long long int`. The correct unsigned type is here named Limb, and must
Expand Down Expand Up @@ -49,6 +52,11 @@ _gmp_clear_func = C_NULL
_mpfr_clear_func = C_NULL

function __init__()
if gmp_version() != GMP_VERSION || gmp_bits_per_limb() != GMP_BITS_PER_LIMB
error(string("the dynamically loaded GMP library (version $(gmp_version()) with __gmp_bits_per_limb == $(gmp_bits_per_limb()))\n",
"does not correspond to the compile time version (version $GMP_VERSION with __gmp_bits_per_limb == $GMP_BITS_PER_LIMB)"))

This comment has been minimized.

Copy link
@tkelman

tkelman Dec 3, 2014

Contributor

I think we have similar checks with blas in interactiveutil.jl, could you look over those and see if it would be worth adding a suggestion for what to do about the problem?

This comment has been minimized.

Copy link
@rfourquet

rfourquet Dec 3, 2014

Author Member

I understood that sys.{so,dll,?} is not yet distributed independently, so won't this check be always OK until then? I wouldn't know what to suggest to do about the problem, as the context is not clear to me.What I could do is adding the GMP version in versioninfo(true).

This comment has been minimized.

Copy link
@tkelman

tkelman Dec 3, 2014

Contributor

I believe this information would also get compiled into the image file sys.ji. The so and dylib's are actually being distributed today. I believe the message about blas integer size mismatch contains a recommendation to delete or regenerate sys.ji and/or so,dll,dylib

This comment has been minimized.

Copy link
@tkelman

tkelman Dec 3, 2014

Contributor

actually that must be some other message, can't remember which right now. the blas warning is at

function check_blas()
and just says to rebuild Julia...

This comment has been minimized.

Copy link
@rfourquet

rfourquet Dec 3, 2014

Author Member

OK I will add "Try to rebuild Julia" to the error message. There are not build option to suggest as for BLAS, but this should just work.

end

global _gmp_clear_func = cglobal((:__gmpz_clear, :libgmp))
global _mpfr_clear_func = cglobal((:mpfr_clear, :libmpfr))
ccall((:__gmp_set_memory_functions, :libgmp), Void,
Expand Down
2 changes: 1 addition & 1 deletion base/random.jl
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ else
immutable RangeGeneratorBigInt <: RangeGenerator
a::BigInt # first
m::BigInt # range length - 1
limbs::Array{Limb} # buffer to be copied into generated BigInt's
limbs::Vector{Limb} # buffer to be copied into generated BigInt's
mask::Limb # applied to the highest limb

RangeGeneratorBigInt(a, m, nlimbs, mask) = new(a, m, Array(Limb, nlimbs), mask)
Expand Down
4 changes: 2 additions & 2 deletions doc/stdlib/base.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4083,7 +4083,7 @@ Random number generation in Julia uses the `Mersenne Twister library <http://www

Most functions related to random generation accept an optional ``AbstractRNG`` as the first argument, ``rng`` , which defaults to the global one if not provided. Morever, some of them accept optionally dimension specifications ``dims...`` (which can be given as a tuple) to generate arrays of random values.

A ``MersenneTwister`` RNG can generate random numbers of the following types: ``Float16, Float32, Float64, Bool, Int16, UInt16, Int32, UInt32, Int64, UInt64, Int128, UInt128`` (or complex numbers or arrays of those types). Random floating point numbers are generated uniformly in [0,1).
A ``MersenneTwister`` RNG can generate random numbers of the following types: ``Float16``, ``Float32``, ``Float64``, ``Bool``, ``Int8``, ``UInt8``, ``Int16``, ``UInt16``, ``Int32``, ``UInt32``, ``Int64``, ``UInt64``, ``Int128``, ``UInt128``, ``BigIng`` (or complex numbers of those types). Random floating point numbers are generated uniformly in [0,1). As ``BigInt`` represents unbounded integers, the interval must be specified (e.g. ``rand(big(1:6))``).

This comment has been minimized.

Copy link
@tkelman

tkelman Dec 3, 2014

Contributor

s/BigIng/BigInt/

This comment has been minimized.

Copy link
@rfourquet

rfourquet Dec 3, 2014

Author Member

Thanks.


.. function:: srand([rng], [seed])

Expand All @@ -4099,7 +4099,7 @@ A ``MersenneTwister`` RNG can generate random numbers of the following types: ``

* an indexable collection (for example ``1:n`` or ``['x','y','z']``), or

* a type: the set of values to pick from is then equivalent to ``typemin(S):typemax(S)`` for integers, and to [0,1) for floating point numbers;
* a type: the set of values to pick from is then equivalent to ``typemin(S):typemax(S)`` for integers (this is not applicable to ``BigInt``), and to [0,1) for floating point numbers;

``S`` defaults to ``Float64``.

Expand Down

3 comments on commit 42fd6b1

@ViralBShah
Copy link
Member

Choose a reason for hiding this comment

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

Will you be creating a PR for this?

@rfourquet
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't intend to... it's such a small change, so I linked to this branch in #9122, as a follow-up. Should I create a PR?

@ViralBShah
Copy link
Member

Choose a reason for hiding this comment

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

No, it isn't necessary - I realized it was on the same branch, and was just wondering how you were planning to incorporate into master.

Please sign in to comment.