From 041ee949084f6ce84618d1b5b8e46d091b6f4e80 Mon Sep 17 00:00:00 2001 From: Juri Date: Tue, 23 Aug 2016 21:10:58 +0200 Subject: [PATCH 1/4] Fix Issue #427 Use Fastpath without random-call when only a single value is allowed to be returned. Added test that result of Next() is within boundaries. --- src/Numerics/Random/RandomSource.cs | 13 +++++++++++++ src/UnitTests/Random/RandomTests.cs | 23 +++++++++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/src/Numerics/Random/RandomSource.cs b/src/Numerics/Random/RandomSource.cs index a3ca0deb..ef776014 100644 --- a/src/Numerics/Random/RandomSource.cs +++ b/src/Numerics/Random/RandomSource.cs @@ -472,6 +472,12 @@ namespace MathNet.Numerics.Random /// protected virtual long DoSampleInt64WithNBits(int bitCount) { + // Fast case: Only 0 is allowed to be returned + // No random call is needed + if (bitCount == 0) + { + return 0; + } var bytes = new byte[8]; DoSampleBytes(bytes); @@ -489,6 +495,13 @@ namespace MathNet.Numerics.Random /// The exclusive upper bound of the random number returned. protected virtual int DoSampleInteger(int maxExclusive) { + // Fast case: Only a single number is allowed to be returned + // No random call is needed + if (maxExclusive == 1) + { + return 0; + } + // non-biased implementation // (biased: return (int)(DoSample() * maxExclusive);) diff --git a/src/UnitTests/Random/RandomTests.cs b/src/UnitTests/Random/RandomTests.cs index 408ff02d..2e1d5ab5 100644 --- a/src/UnitTests/Random/RandomTests.cs +++ b/src/UnitTests/Random/RandomTests.cs @@ -78,6 +78,29 @@ namespace MathNet.Numerics.UnitTests.Random Assert.IsTrue(sum >= (N/2.0) - (.05*N)); Assert.IsTrue(sum <= (N/2.0) + (.05*N)); + var disposable = random as IDisposable; + if (disposable != null) + { + disposable.Dispose(); + } + } + + /// + /// Next() result is in boundaries. + /// + [Test] + public void Boundaries() + { + var random = (System.Random)Activator.CreateInstance(_randomType, new object[] { false }); + + for (var i = 1; i < N; i++) + { + var j = N; + var next = random.Next(i, j); + Assert.IsTrue(next >= i, string.Format("Value {0} is smaller than lower bound {1}", next, i)); + Assert.IsTrue(next < j, string.Format("Value {0} is larger or equal to upper bound {1}", next, j)); + } + var disposable = random as IDisposable; if (disposable != null) { From d852f991200b4dc1a1f9530608d207b9fc5cb6ae Mon Sep 17 00:00:00 2001 From: Juri Date: Tue, 23 Aug 2016 21:14:22 +0200 Subject: [PATCH 2/4] Fix Issue #427 Added fix to DoSampleInt32WithNBits too --- src/Numerics/Random/RandomSource.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Numerics/Random/RandomSource.cs b/src/Numerics/Random/RandomSource.cs index ef776014..33f66653 100644 --- a/src/Numerics/Random/RandomSource.cs +++ b/src/Numerics/Random/RandomSource.cs @@ -455,6 +455,12 @@ namespace MathNet.Numerics.Random /// protected virtual int DoSampleInt32WithNBits(int bitCount) { + // Fast case: Only 0 is allowed to be returned + // No random call is needed + if (bitCount == 0) + { + return 0; + } var bytes = new byte[4]; DoSampleBytes(bytes); From 8e131b481bdc100c6a94e7658a60f73b95c49b7f Mon Sep 17 00:00:00 2001 From: Christoph Ruegg Date: Tue, 6 Sep 2016 11:37:30 +0200 Subject: [PATCH 3/4] Random: clarify ranges, ensure them at the call site #427 --- src/Numerics/Numerics.csproj | 12 +- src/Numerics/Properties/Resources.Designer.cs | 9 ++ src/Numerics/Properties/Resources.resx | 57 ++++----- src/Numerics/Random/RandomSource.cs | 108 ++++++++++++++---- 4 files changed, 130 insertions(+), 56 deletions(-) diff --git a/src/Numerics/Numerics.csproj b/src/Numerics/Numerics.csproj index f19b464f..0063697a 100644 --- a/src/Numerics/Numerics.csproj +++ b/src/Numerics/Numerics.csproj @@ -167,6 +167,11 @@ + + True + True + Resources.resx + @@ -428,11 +433,6 @@ - - True - True - Resources.resx - @@ -472,8 +472,8 @@ PublicResXFileCodeGenerator - Resources.Designer.cs Designer + Resources.Designer.cs diff --git a/src/Numerics/Properties/Resources.Designer.cs b/src/Numerics/Properties/Resources.Designer.cs index b4650370..ca56ef13 100644 --- a/src/Numerics/Properties/Resources.Designer.cs +++ b/src/Numerics/Properties/Resources.Designer.cs @@ -314,6 +314,15 @@ namespace MathNet.Numerics.Properties { } } + /// + /// Looks up a localized string similar to In the specified range, the exclusive maximum must be greater than the inclusive minimum.. + /// + public static string ArgumentMaxExclusiveMustBeLargerThanMinInclusive { + get { + return ResourceManager.GetString("ArgumentMaxExclusiveMustBeLargerThanMinInclusive", resourceCulture); + } + } + /// /// Looks up a localized string similar to In the specified range, the minimum is greater than maximum.. /// diff --git a/src/Numerics/Properties/Resources.resx b/src/Numerics/Properties/Resources.resx index e3c53791..13cc7a6e 100644 --- a/src/Numerics/Properties/Resources.resx +++ b/src/Numerics/Properties/Resources.resx @@ -1,17 +1,17 @@  - @@ -282,6 +282,9 @@ In the specified range, the minimum is greater than maximum. + + In the specified range, the exclusive maximum must be greater than the inclusive minimum. + The upper bound must be at least as large as the lower bound. diff --git a/src/Numerics/Random/RandomSource.cs b/src/Numerics/Random/RandomSource.cs index 33f66653..c9089f4c 100644 --- a/src/Numerics/Random/RandomSource.cs +++ b/src/Numerics/Random/RandomSource.cs @@ -3,7 +3,7 @@ // http://numerics.mathdotnet.com // http://github.com/mathnet/mathnet-numerics // -// Copyright (c) 2009-2015 Math.NET +// Copyright (c) 2009-2016 Math.NET // // Permission is hereby granted, free of charge, to any person // obtaining a copy of this software and associated documentation @@ -133,28 +133,39 @@ namespace MathNet.Numerics.Random return DoSampleInteger(); } } - - return DoSampleInteger(); + else + { + return DoSampleInteger(); + } } /// /// Returns a random number less then a specified maximum. /// - /// The exclusive upper bound of the random number returned. + /// The exclusive upper bound of the random number returned. Range: maxExclusive ≥ 1. /// A 32-bit signed integer less than . - /// is negative. + /// is zero or negative. public sealed override int Next(int maxExclusive) { + // Invalid case: Zero and less are not valid use cases. if (maxExclusive <= 0) { throw new ArgumentException(Resources.ArgumentMustBePositive); } + // Fast case: Only zero is allowed to be returned. No sampling is needed. + if (maxExclusive == 1) + { + return 0; + } + + // Simple case: standard range if (maxExclusive == int.MaxValue) { return Next(); } + // Sample with 1 < maxExclusive < int.MaxValue if (_threadSafe) { lock (_lock) @@ -162,28 +173,39 @@ namespace MathNet.Numerics.Random return DoSampleInteger(maxExclusive); } } - - return DoSampleInteger(maxExclusive); + else + { + return DoSampleInteger(maxExclusive); + } } /// /// Returns a random number within a specified range. /// /// The inclusive lower bound of the random number returned. - /// The exclusive upper bound of the random number returned. must be greater than or equal to . + /// The exclusive upper bound of the random number returned. Range: maxExclusive > minExclusive. /// /// A 32-bit signed integer greater than or equal to and less than ; that is, the range of return values includes but not . If equals , is returned. /// /// is greater than . public sealed override int Next(int minInclusive, int maxExclusive) { - if (minInclusive > maxExclusive) + // Invalid case: empty range. + if (minInclusive >= maxExclusive) { - throw new ArgumentException(Resources.ArgumentMinValueGreaterThanMaxValue); + throw new ArgumentException(Resources.ArgumentMaxExclusiveMustBeLargerThanMinInclusive); + } + + // Fast case: Only minInclusive is allowed to be returned. No sampling is needed. + if (maxExclusive == minInclusive + 1) + { + return minInclusive; } + // Simple case: simple range if (minInclusive == 0) { + // Simple case: standard range if (maxExclusive == int.MaxValue) { return Next(); @@ -192,6 +214,7 @@ namespace MathNet.Numerics.Random return Next(maxExclusive); } + // Sample with minInclusive + 1 < maxExclusive if (_threadSafe) { lock (_lock) @@ -199,8 +222,10 @@ namespace MathNet.Numerics.Random return DoSampleInteger(minInclusive, maxExclusive); } } - - return DoSampleInteger(minInclusive, maxExclusive); + else + { + return DoSampleInteger(minInclusive, maxExclusive); + } } /// @@ -243,15 +268,30 @@ namespace MathNet.Numerics.Random /// Fills an array with random numbers within a specified range. /// /// The array to fill with random values. - /// The exclusive upper bound of the random number returned. + /// The exclusive upper bound of the random number returned. Range: maxExclusive ≥ 1. public void NextInt32s(int[] values, int maxExclusive) { + // Invalid case: Zero and less are not valid use cases. + if (maxExclusive <= 0) + { + throw new ArgumentException(Resources.ArgumentMustBePositive); + } + + // Fast case: Only zero is allowed to be returned. No sampling is needed. + if (maxExclusive == 1) + { + Array.Clear(values, 0, values.Length); + return; + } + + // Simple case: standard range if (maxExclusive == int.MaxValue) { NextInt32s(values); return; } + // Sample with 1 < maxExclusive < int.MaxValue if (_threadSafe) { lock (_lock) @@ -271,27 +311,46 @@ namespace MathNet.Numerics.Random } } + /// + /// Returns an array with random 32-bit signed integers within the specified range. + /// + /// The size of the array to fill. + /// The exclusive upper bound of the random number returned. Range: maxExclusive ≥ 1. + public int[] NextInt32s(int count, int maxExclusive) + { + var values = new int[count]; + NextInt32s(values, maxExclusive); + return values; + } + /// /// Fills an array with random numbers within a specified range. /// /// The array to fill with random values. /// The inclusive lower bound of the random number returned. - /// The exclusive upper bound of the random number returned. must be greater than or equal to . + /// The exclusive upper bound of the random number returned. Range: maxExclusive > minExclusive. public void NextInt32s(int[] values, int minInclusive, int maxExclusive) { - if (minInclusive > maxExclusive) + // Invalid case: empty range. + if (minInclusive >= maxExclusive) { - throw new ArgumentException(Resources.ArgumentMinValueGreaterThanMaxValue); + throw new ArgumentException(Resources.ArgumentMaxExclusiveMustBeLargerThanMinInclusive); } - if (maxExclusive == int.MaxValue && minInclusive == 0) + // Fast case: Only minInclusive is allowed to be returned. No sampling is needed. + if (maxExclusive == minInclusive + 1) { - NextInt32s(values); + for (var i = 0; i < values.Length; i++) + { + values[i] = minInclusive; + } return; } + // Simple case: simple range if (minInclusive == 0) { + // Simple case: standard range if (maxExclusive == int.MaxValue) { NextInt32s(values); @@ -302,6 +361,7 @@ namespace MathNet.Numerics.Random return; } + // Sample with minInclusive + 1 < maxExclusive if (_threadSafe) { lock (_lock) @@ -326,7 +386,7 @@ namespace MathNet.Numerics.Random /// /// The size of the array to fill. /// The inclusive lower bound of the random number returned. - /// The exclusive upper bound of the random number returned. must be greater than or equal to . + /// The exclusive upper bound of the random number returned. Range: maxExclusive > minExclusive. public int[] NextInt32s(int count, int minInclusive, int maxExclusive) { var values = new int[count]; @@ -359,7 +419,7 @@ namespace MathNet.Numerics.Random /// Returns an infinite sequence of random numbers within a specified range. /// /// The inclusive lower bound of the random number returned. - /// The exclusive upper bound of the random number returned. must be greater than or equal to . + /// The exclusive upper bound of the random number returned. Range: maxExclusive > minExclusive. public IEnumerable NextInt32Sequence(int minInclusive, int maxExclusive) { if (minInclusive > maxExclusive) @@ -461,6 +521,7 @@ namespace MathNet.Numerics.Random { return 0; } + var bytes = new byte[4]; DoSampleBytes(bytes); @@ -484,6 +545,7 @@ namespace MathNet.Numerics.Random { return 0; } + var bytes = new byte[8]; DoSampleBytes(bytes); @@ -498,7 +560,7 @@ namespace MathNet.Numerics.Random /// /// Returns a random 32-bit signed integer within the specified range. /// - /// The exclusive upper bound of the random number returned. + /// The exclusive upper bound of the random number returned. Range: maxExclusive ≥ 1. protected virtual int DoSampleInteger(int maxExclusive) { // Fast case: Only a single number is allowed to be returned @@ -507,7 +569,7 @@ namespace MathNet.Numerics.Random { return 0; } - + // non-biased implementation // (biased: return (int)(DoSample() * maxExclusive);) @@ -535,7 +597,7 @@ namespace MathNet.Numerics.Random /// Returns a random 32-bit signed integer within the specified range. /// /// The inclusive lower bound of the random number returned. - /// The exclusive upper bound of the random number returned. must be greater than or equal to . + /// The exclusive upper bound of the random number returned. Range: maxExclusive > minExclusive. protected virtual int DoSampleInteger(int minInclusive, int maxExclusive) { return DoSampleInteger(maxExclusive - minInclusive) + minInclusive; From 5b236aee767f2e7c27daba8e3dc730b4c6ce5350 Mon Sep 17 00:00:00 2001 From: Christoph Ruegg Date: Tue, 6 Sep 2016 11:59:11 +0200 Subject: [PATCH 4/4] Random: drop redundant range check in inner loop implementation #427 --- src/Numerics/Random/RandomSource.cs | 20 +++++++------------- src/Numerics/Random/SystemRandomSource.cs | 9 ++++++++- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/Numerics/Random/RandomSource.cs b/src/Numerics/Random/RandomSource.cs index c9089f4c..cce5a1f8 100644 --- a/src/Numerics/Random/RandomSource.cs +++ b/src/Numerics/Random/RandomSource.cs @@ -165,7 +165,7 @@ namespace MathNet.Numerics.Random return Next(); } - // Sample with 1 < maxExclusive < int.MaxValue + // Sample with maxExclusive ≥ 2 if (_threadSafe) { lock (_lock) @@ -214,7 +214,7 @@ namespace MathNet.Numerics.Random return Next(maxExclusive); } - // Sample with minInclusive + 1 < maxExclusive + // Sample with maxExclusive ≥ minExclusive + 2 if (_threadSafe) { lock (_lock) @@ -291,7 +291,7 @@ namespace MathNet.Numerics.Random return; } - // Sample with 1 < maxExclusive < int.MaxValue + // Sample with maxExclusive ≥ 2 if (_threadSafe) { lock (_lock) @@ -361,7 +361,7 @@ namespace MathNet.Numerics.Random return; } - // Sample with minInclusive + 1 < maxExclusive + // Sample with maxExclusive ≥ minExclusive + 2 if (_threadSafe) { lock (_lock) @@ -560,16 +560,9 @@ namespace MathNet.Numerics.Random /// /// Returns a random 32-bit signed integer within the specified range. /// - /// The exclusive upper bound of the random number returned. Range: maxExclusive ≥ 1. + /// The exclusive upper bound of the random number returned. Range: maxExclusive ≥ 2 (not verified, must be ensured by caller). protected virtual int DoSampleInteger(int maxExclusive) { - // Fast case: Only a single number is allowed to be returned - // No random call is needed - if (maxExclusive == 1) - { - return 0; - } - // non-biased implementation // (biased: return (int)(DoSample() * maxExclusive);) @@ -597,9 +590,10 @@ namespace MathNet.Numerics.Random /// Returns a random 32-bit signed integer within the specified range. /// /// The inclusive lower bound of the random number returned. - /// The exclusive upper bound of the random number returned. Range: maxExclusive > minExclusive. + /// The exclusive upper bound of the random number returned. Range: maxExclusive ≥ minExclusive + 2 (not verified, must be ensured by caller). protected virtual int DoSampleInteger(int minInclusive, int maxExclusive) { + // Sample with maxExclusive ≥ 2 return DoSampleInteger(maxExclusive - minInclusive) + minInclusive; } } diff --git a/src/Numerics/Random/SystemRandomSource.cs b/src/Numerics/Random/SystemRandomSource.cs index 04c0dee0..e7554c3c 100644 --- a/src/Numerics/Random/SystemRandomSource.cs +++ b/src/Numerics/Random/SystemRandomSource.cs @@ -126,6 +126,10 @@ namespace MathNet.Numerics.Random return _random.Next(); } + /// + /// Returns a random 32-bit signed integer within the specified range. + /// + /// The exclusive upper bound of the random number returned. Range: maxExclusive ≥ 2 (not verified, must be ensured by caller). protected override int DoSampleInteger(int maxExclusive) { return _random.Next(maxExclusive); @@ -135,12 +139,15 @@ namespace MathNet.Numerics.Random /// Returns a random 32-bit signed integer within the specified range. /// /// The inclusive lower bound of the random number returned. - /// The exclusive upper bound of the random number returned. must be greater than or equal to . + /// The exclusive upper bound of the random number returned. Range: maxExclusive ≥ minExclusive + 2 (not verified, must be ensured by caller). protected override int DoSampleInteger(int minInclusive, int maxExclusive) { return _random.Next(minInclusive, maxExclusive); } + /// + /// Fills the elements of a specified array of bytes with random numbers in full range, including zero and 255 (). + /// protected override void DoSampleBytes(byte[] buffer) { _random.NextBytes(buffer);