-
-
Notifications
You must be signed in to change notification settings - Fork 1k
Add support for generating pseudorandom numbers using MT19937 #204
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
Conversation
|
Wow, thanks so much for working on this, greatly appreciated! |
| --> | ||
|
|
||
| # mt19937 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/mt19937/Mersenne Twister/
|
|
||
| ```javascript | ||
| var max = mt19937.MAX; | ||
| // returns 2147483646 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be updated to 4294967295
|
|
||
| {{alias}}() | ||
| Returns a pseudorandom integer on the interval `[1, 2147483646]`. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know the period of MT19937? I believe this is documented somewhere. And if so, we should include the period in the documentation, both here and in the README, as done for minstd.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I put the period of MT19937 in this file. Not sure why this is still showing as changes requested.
| Examples | ||
| -------- | ||
| > var v = {{alias}}.MAX | ||
| 2147483646 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be 4294967295
| var s; | ||
|
|
||
| if ( arguments.length ) { | ||
| if ( !isPositiveInteger( seed ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you still planning on adding support for a seed array?
| * // returns <number> | ||
| */ | ||
| function normalized() { | ||
| return ( mt19937() - 1 ) * ( 1.0 / 4294967295.0 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would define a normalization constant and elevate to the top level scope. Otherwise, we have to assume that the compiler will be smart enough to not repeatedly evaluate a constant.
| // MAIN // | ||
|
|
||
| /** | ||
| * Generates a pseudorandom integer on the interval \\( [1,2^{31}-1) \\). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Range should be updated.
| v = mt19937(); | ||
| t.equal( typeof v, 'number', 'returns a number' ); | ||
| t.equal( isPositiveInteger( v ), true, 'returns a positive integer' ); | ||
| t.equal( v >= 1 && v <= UINT32_MAX-1, true, 'returns an integer between 1 and 2^31-1 (inclusive)' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test description should be updated.
|
|
||
| tape( 'attached to the returned function is the maximum possible generated number', function test( t ) { | ||
| var mt19937 = factory(); | ||
| t.equal( mt19937.MAX, UINT32_MAX-1, 'has `MAX` property' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is the max UINT32_MAX-1? According to the C source, I might expect UINT32_MAX.
UINT32_MAX == 0xffffffff
| }); | ||
|
|
||
| tape( 'attached to the main export is the maximum possible generated number', function test( t ) { | ||
| t.equal( mt19937.MAX, UINT32_MAX-1, 'has `MAX` property' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same questions and comments as for the factory tests.
| setReadOnly( normalized, 'SEED', mt19937.SEED ); | ||
| setReadOnly( normalized, 'MIN', ( mt19937.MIN - 1.0 ) / NORMALIZATION_CONSTANT ); | ||
| setReadOnly( normalized, 'MAX', ( 4294967294.0 ) * ( 1.0 / 4294967295.0 ) ); | ||
| setReadOnly( normalized, 'MIN', ((( mt19937.MIN - 1.0 )) >>> 0) / ( NORMALIZATION_CONSTANT >>> 0 ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the rationale for ensuring uint32 operands here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait...when you do
( mt19937.MAX - 1.0 ) / NORMALIZATION_CONSTANTyou get back an integer value? Something seems off. The above, if MAX is 4294967295, should return a value between 0 and 1. If it returns an integer, something seems amiss. Can you provide the exact code which generates -(UINT32_MAX-1)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setReadOnly( normalized, 'MAX', ((( mt19937.MAX - 1.0 )) ) / ( NORMALIZATION_CONSTANT ) );
This gives the error shown earlier.
| */ | ||
| function normalized() { | ||
| return ( mt19937() - 1 ) * ( 1.0 / 4294967295.0 ); | ||
| return ((( mt19937() - 1.0 )) >>> 0) / (NORMALIZATION_CONSTANT >>> 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the rationale for ensuring uint32 operands, here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same reason as above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah...another thing. The normalized algo should be the algo for generating 53 bits of randomness, not just 32 bits of randomness. That corresponds to function genrand_res53 in the source. The approach is slightly deficient, as documented in a paper linked to in the RFC, but fine for most purposes.
| } | ||
| if ( seed > MAX_SEED ) { | ||
| throw new RangeError( 'invalid input argument. Must provide a positive integer less than the maximum signed 32-bit integer. Value: `' + seed + '`.' ); | ||
| if ( Array.isArray( seed ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the @stdlib/assert/is-array assertion utility to ensure support for older environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, upon reflection, we need not restrict seeding to "generic" arrays. A user may want to provide a typed array, for example. So switch the assertion to isCollection (see @stdlib/assert/is-collection, which allows array-like objects, generic arrays, and typed arrays).
| function factory( seed ) { | ||
| var seedIsArray = false; | ||
| var state; | ||
| var mag01 = [ 0x0, MATRIX_A ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be moved to the parent scope.
| seedIsArray = true; | ||
| i = 1; | ||
| j = 0; | ||
| k = ( N > seed.length ) ? N : seed.length; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the @stdlib/math/base/special/max pkg and then k = max( N, seed.length ); in order to better convey intent.
| var state; | ||
| var mag01 = [ 0x0, MATRIX_A ]; | ||
| var mti; | ||
| var mt = new Array( N ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Declare mt but do not assign here. Instead, move initialization to L95 or thereabout. Why? Otherwise, if provided a bad input, we'll have unnecessarily allocated memory, just making the situation worse.
| } | ||
| state = seed | 0; // asm type annotation | ||
| } else { | ||
| state = randint32() | 0; // asm type annotation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While we generated a "signed" 32-bit integer for LCG, is that necessary here? Can we not do randuint32? If so, the asm.js type annotations would need to be updated. In short, is the seed allowed to exceed INT32_MAX, but must be less than or equal to UINT32_MAX? If so, we should update the seeding procedure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the seed can be more than INT32_MAX and less than or equal to UINT32_MAX. So we can do randuint32. But does using a uint32 random seed over int32 random seed make much of a difference? The original C implementation uses a hardcoded seed (init_genrand(5489UL); /* a default initial seed is used */) if not given one.
Also, I never got around the // asm type annotation lines. I just followed the pattern. What exactly is happening there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, seeing randint32 makes me believe it is "intentional" (as if the algo must use int32), rather than "accidental". I think there is some mismatch based on the fact that the seed "array", as defined in the source, is uint32 (unsigned long) and then an int32 seed is used here. As for the hardcoded seed, I would avoid this and keep random generation of a seed in order to avoid automatic deterministic output, thus requiring "opt-in", rather than "opt-out" for repeated sequences. Personally, I would just amend to randuint32, but am open to counterarguments.
Re: asm type annotations. These come from asm.js, the precursor to WebAssembly, which allow us to give the compiler hints as to numeric values. While JavaScript officially only has numeric values of type double, in practice, JavaScript has both signed and unsigned integers, but we need to indicate them as such to the compiler. So anytime we use |0 or >>>0 for indicating numeric types, we always add a // asm type annotation comment so future devs understand why the, on the surface, no-op bit operations are included in the source. They are there to offer the compiler hints so that compilers can perform various performance optimizations (e.g., guaranteed integer opts).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so basically all the |0 need to be changed to >>>0?
function randuint32() {
var v = floor( 1.0 + (MAX*Math.random()) ); // eslint-disable-line stdlib/no-builtin-math
return v>>>0; // asm type annotation
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, wherever uint32 applies.
| } | ||
|
|
||
| if ( seedIsArray ) { | ||
| for (; k > 0; k--) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To nitpick here, we tend to emphasize consistency of style. At L96, we include spaces around the elements within the loop condition. Here, and below, those spaces are omitted. Our preference is to use spaces.
| mt[i] >>>= 0; | ||
| i += 1; | ||
| j += 1; | ||
| if (i >= N) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar rationale for including spaces within the conditional.
|
|
||
| for (k = N - 1; k; k--) { | ||
| s = mt[i - 1] ^ (mt[i - 1] >>> 30); | ||
| mt[i] = (mt[i] ^ (((((s & 0xffff0000) >>> 16) * 1566083941) << 16) + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our preference is not to split lines. Instead, we tend to disable the lint rule.
// eslint-disable-line max-len
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to add the following to the license header for this file:
*
*
* ## Notice
*
* The original C code and copyright notice are from the [source implementation]{@link http://www.math.sci.hiroshima-u.ac.jp/~m-mat/MT/MT2002/CODES/mt19937ar.c}. The implementation has been modified for JavaScript.
*
* ```text
* Copyright (C) 1997 - 2002, Makoto Matsumoto and Takuji Nishimura,
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
*
* 1. Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
*
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
*
* 3. The names of its contributors may not be used to endorse or promote
* products derived from this software without specific prior written
* permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
* CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
* EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
* PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
* PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
* LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
* NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
* SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
* ```
*/Under the terms of BSD-3, as shown above, we need to include the copyright notice and license as a "translation" is considered a modification. See this for an example where we do something similar for Boost.
|
|
||
| var NORMALIZATION_CONSTANT = (UINT32_MAX) | 0; // asm type annotation | ||
| var MAX_SEED = (INT32_MAX - 1) | 0; // asm type annotation | ||
| var NORMALIZATION_CONSTANT = (UINT32_MAX) >>> 0; // asm type annotation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. This was why the normalized MAX was returning the wrong number. |0 converted UINT32_MAX to a signed 32-bit integer.
(UINT32_MAX) | 0 === -1As this has been fixed, you should not need to ensure uint32 operands when setting the normalized properties.
| } | ||
| if ( seed > MAX_SEED ) { | ||
| throw new RangeError( 'invalid argument. Must provide a positive integer less than the maximum signed 32-bit integer. Value: `' + seed + '`.' ); | ||
| throw new RangeError( 'invalid argument. Must provide a positive integer less than the maximum unsigned 32-bit integer. Value: `' + seed + '`.' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
less than or equal to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry. Disregard this. Although why is the max seed UINT32_MAX - 1? Why cannot it not be UINT32_MAX?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I overlooked the fact that UINT32_MAX is 2^{32} - 1 and not 2^{32}.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries. Thanks for clarifying!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the noisy PR, could have done a better job. But does everything look okay now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No worries! In all, awesome work. Especially for things which have added complexity, we can expect a bit more back and forth.
Re: status. Will give another detailed look shortly.
| return ((( mt19937() - 1.0 )) >>> 0) / (NORMALIZATION_CONSTANT >>> 0); | ||
| var x = mt19937() >>> 5; | ||
| var y = mt19937() >>> 6; | ||
| return ( ( x * 67108864.0 ) + y ) * ( 1.0 / 9007199254740992.0 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can avoid at least one magic number here. The normalization constant in L237 is one over the maximum safe integer for doubles (@stdlib/constants/math/float64-max-safe-integer) plus 1. Can move to top level scope as
var NORMALIZED_NORMALIZATION_CONSTANT = 1.0 / ( FLOAT64_MAX_SAFE_INTEGER + 1.0 );|
|
||
| /** | ||
| * Returns a pseudorandom integer on the interval \\([1, 2^{31}-1)\\). | ||
| * Returns a pseudorandom integer on the interval \\([1, 2^{32})\\). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2^{32} - 1. You subtract one at L29 and then you add one back in L45. As MAX*Math.random() cannot exceed UINT32_MAX - 1, then adding one cannot exceed UINT32_MAX, which is equal to 2^{32} - 1.
|
|
||
| #### mt19937() | ||
|
|
||
| Returns a pseudorandom integer on the interval `[1, 2147483646]`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The range needs to be updated.
| var rand = mt19937.factory(); | ||
| ``` | ||
|
|
||
| By default, a random integer is used to seed the returned generator. To seed the generator, provide an `integer` on the interval `[1, 2147483646]`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The range needs to be updated.
|
|
||
| ## Notes | ||
|
|
||
| - The generator has a period of `2^{19937} - 1` (see [Wikipedia page][mt]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove the "(see Wikipedia page)" parenthetical.
| @@ -0,0 +1,96 @@ | |||
|
|
|||
| {{alias}}() | |||
| Returns a pseudorandom integer on the interval `[1, 2147483646]`. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The range needs to be updated.
| Parameters | ||
| ---------- | ||
| seed: integer (optional) | ||
| Pseudorandom number generator seed. Must be on the interval |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The range needs to be updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did this, not sure why this is not "outdated".
| * | ||
| * @param {PositiveInteger} [seed] - pseudorandom number generator seed | ||
| * @throws {TypeError} must provide a positive integer | ||
| * @throws {RangeError} must provide a positive integer less than the maximum signed 32-bit integer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/signed/unsigned/
| if ( !isPositiveInteger( seed ) ) { | ||
| throw new TypeError( 'invalid argument. Must provide a positive integer. Value: `' + seed + '`.' ); | ||
| } | ||
| if ( seed >= MAX_SEED ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I asked this before, but what is the reason that seed cannot equal UINT32_MAX?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be equal. I changed the value of MAX_SEED constant defined above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realize that the constant MAX_SEED need not be defined and can be simply replaced by UINT32_MAX. But I reckon it's more readable and obvious with the extra variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood. Am okay with that.
| }); | ||
|
|
||
| tape( 'attached to the `normalized` method is the maximum possible generated number', function test( t ) { | ||
| t.equal( mt19937.normalized.MAX, ((UINT32_MAX-1.0))/(UINT32_MAX), 'has `MAX` property' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this is correct. We now use 53-bits of precision, not the 32 bits of precision algo.
|
|
||
| tape( 'attached to the `normalized` method is the maximum possible generated number', function test( t ) { | ||
| var mt19937 = factory(); | ||
| t.equal( mt19937.normalized.MAX, (UINT32_MAX-1.0)/(UINT32_MAX), 'has `MAX` property' ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this is correct. We now use the 53 bits of precision algo, not the 32 bits of precision algo.
| t.end(); | ||
| }); | ||
|
|
||
| tape( 'the function returns pseudorandom numbers from the Mersenne Twister PRNG', function test( t ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference between this test and the test beginning at L144?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry about that, when I added the test for array seed, I must have duplicated the integer seed test.
|
|
||
| // MODULES // | ||
|
|
||
| var max = require('@stdlib/math/base/special/max'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The spacing around the require arguments is inconsistent with the spacing used elsewhere in the package. For consistency, space before and after the require path.
| * @param {PositiveInteger} [seed] - pseudorandom number generator seed | ||
| * @throws {TypeError} must provide a positive integer | ||
| * @throws {RangeError} must provide a positive integer less than the maximum signed 32-bit integer | ||
| * @throws {RangeError} must provide a positive integer less than the maximum unsigned 32-bit integer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
less than or equal to
| setReadOnly( normalized, 'SEED', mt19937.SEED ); | ||
| setReadOnly( normalized, 'MIN', ( mt19937.MIN - 1.0 ) / ( NORMALIZATION_CONSTANT ) ); | ||
| setReadOnly( normalized, 'MAX', ( mt19937.MAX - 1.0 ) / ( NORMALIZATION_CONSTANT ) ); | ||
| setReadOnly( normalized, 'MIN', 0 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
0.0
| setReadOnly( normalized, 'MIN', ( mt19937.MIN - 1.0 ) / ( NORMALIZATION_CONSTANT ) ); | ||
| setReadOnly( normalized, 'MAX', ( mt19937.MAX - 1.0 ) / ( NORMALIZATION_CONSTANT ) ); | ||
| setReadOnly( normalized, 'MIN', 0 ); | ||
| setReadOnly( normalized, 'MAX', ( ( ( UINT32_MAX >>> 5 ) * 67108864.0 ) + ( UINT32_MAX >>> 6 ) ) * NORMALIZED_NORMALIZATION_CONSTANT ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
| } | ||
| }); | ||
|
|
||
| tape( 'the function throws a range error if provided an integer greater than or equal to the maximum unsigned 32-bit integer', function test( t ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test should be updated.
kgryte
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ShraddheyaS Everything looks good. Just need to change one test description and should be ready for merge!
| t.end(); | ||
| }); | ||
|
|
||
| tape( 'attached to the main export is a method to generate linear congruential pseudorandom number generator', function test( t ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test description should be updated.
|
@kgryte Thank you for your patience. I went ahead and also added a test for the normalized method as well. |
|
@ShraddheyaS No worries! This is part of the process. :) Thanks for all your work and your persistence. This is a great addition to the project and greatly appreciated. :) |

Resolves #202.
Checklist
develop.developbranch.Description
This pull request:
Related Issues
This pull request:
Questions
No.
Other
No.
@stdlib-js/reviewers