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: Reintroduce random BigFloats removed in 1a8885b #13950

Closed
wants to merge 1 commit into from

Conversation

mschauer
Copy link
Contributor

These would be the bare bones changes needed to reintroduce sampling of random floats to address #13948 (Tests and docs if interest, then.)

This does not yet intent to link the BigFloat-number generator with the global number generator. This can be done similar to #4845, but I think linking those is not strictly necessary.

This order of dependencies (GMPRandState in gmp.jl, mpfr_urandomb-wrapper in mpfr.jl depending on GMPRandState and finally BigFloatRNG in random.jl depending on both of them) corresponds with the order in sysimg.jl. Would that mean that the problem 1a8885b does not appear?

@mschauer mschauer changed the title WIP: Reintroduce random BigFloats removed in 4cc21ca WIP: Reintroduce random BigFloats removed in 1a8885b Nov 11, 2015
@mschauer
Copy link
Contributor Author

Feedback welcome

@tkelman
Copy link
Contributor

tkelman commented Nov 26, 2015

Implementation looks okay to me, but does need tests and docs.

@mschauer
Copy link
Contributor Author

Thanks. As said, if there is general interests, I add the tests and the docs.

@rfourquet
Copy link
Member

Naive question (I don't know how mpfr works) : are there too many subtleties to avoid manipulating directly the BigFloat type and filling its d field with a Julia RNG? (Similar to generating 52 random bits and masking properly for random Float64, and substracting 1)

@ViralBShah ViralBShah added the randomness Random number generation and the Random stdlib label Dec 1, 2015
@ViralBShah ViralBShah added this to the 0.5.0 milestone Dec 1, 2015
@StefanKarpinski
Copy link
Member

What @rfourquet is suggesting seems plausible to me – the MPFR data structure is well-defined and not terribly complicated and we have access to it on the Julia side as well.

@tkelman tkelman added needs tests Unit tests are required for this change needs docs Documentation for this change is required labels Dec 2, 2015
@mschauer
Copy link
Contributor Author

Triage: I admit I won't implement the direct direct MPFR float construction in the next future, but I can add tests and docs for this approach. If somebody wants to take on @rfourquet s proposal I assist of course.

@tkelman
Copy link
Contributor

tkelman commented Apr 21, 2016

Given the lack of activity here I don't think this should hold up the 0.5 release. It's a new (reintroduced) feature so it'll happen when it happens.

@tkelman tkelman removed this from the 0.5.0 milestone Apr 21, 2016
@mschauer
Copy link
Contributor Author

Closing this (dormant) PR, the corresponding issue is open for reference.

@mschauer mschauer closed this Jun 25, 2017
@rfourquet rfourquet added the bignums BigInt and BigFloat label Jul 9, 2017
@rfourquet
Copy link
Member

The work you did here is worth keeping somewhere in my opinion, either in base or a package (e.g. https://github.com/sunoru/RandomNumbers.jl/ if the author welcomes contributions). My preference is for base, but I guess unlikely to happen, as non-essential (and base may eventually not depend on GMP anymore). I would happily assist extending your code to make it into a full fledged RNG.

@rfourquet rfourquet mentioned this pull request Dec 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bignums BigInt and BigFloat needs docs Documentation for this change is required needs tests Unit tests are required for this change randomness Random number generation and the Random stdlib
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants