Add support for rand's Uniform distribution to all Vec types
#580
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…hich will be proxies to UniformInt's impls; plus, remove UniformFloatVec*
…cro, and additionally change a bunch of vec-type constructors to use the From trait, so that it works across all vec types
…le certain traits on `rand::distributions::Uniform`
This was needed with how I was originally building the impls - but I never thought to remove it afterward.
Float vector types seem to fail when running these tests, due to a range overflow, so more work is needed
|
This looks OK to me so far. |
… of the function We may be able to expand this macro to be used for both int and float purposes, although for now it only covers ints
|
@bitshifter Thank you. I'm working on cleaning up the code currently, and I still need to make tests for float types; but the main functionality is finished. |
* Reuse `int_rng` and `vec_rng` to reduce the amount of code generated * Clarify the __repeat_code comments * `test_uniform` now generates tests for both ref and non-ref versions * Spread macro parameters as we'll be going over 80 characters
…tests for float types; also implements a new `upper_range_multiplier` parameter on the macro to sidestep the "Range overflow" issue I was getting before with floats
|
I believe this PR is ready for review and merging. All vec types (except P.S. After this PR is merged, I will probably look at refactoring/rewriting |
|
Thanks for this! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #579.
This PR makes the following changes to the
glamcrate:rand'sUniformdistribution to all Vec typesimpl_rand.rsfile into their relevant macros, easing readability and maintainabilityThis is still a Work-In-Progress
I have yet to do the following:
I suspect that the code quality could also be improved in some places. If you spot any way to improve it, let me know! (or if you're bitshifter, feel free to commit directly to my PR!)
Some notes
The
UniformVec*structs used to handle uniform distribution are part of a non-public module, and thus cannot be navigated to in the documentation. While I can rectify this upon request, I don't believe it's necessary, since the wayrandintends for the user to performUniformdistribution is through therand::distributions::Uniformtype, rather than using the backend types directly.I have not added (de)serialization support to the
UniformVec*structs. However,randoffers (de)serialization of its distribution structs, so I am unsure if (de)serialization is wanted forUniformVec*.