Skip to content

Commit

Permalink
Fix for non-uniform random value generators (#26)
Browse files Browse the repository at this point in the history
* Added failing unit test

* Fix identified. System.Random requires a lock for multithread access

* Issue resolved. Random values are now uniform

* Moved tests to a dedicated folder
  • Loading branch information
DarekDan committed Nov 30, 2022
1 parent 642978a commit 653818f
Show file tree
Hide file tree
Showing 5 changed files with 184 additions and 25 deletions.
9 changes: 9 additions & 0 deletions src/RandomDataGenerator Solution.sln
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "RandomDataGenerator.Gui3",
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ConsoleApp", "ConsoleApp\ConsoleApp.csproj", "{13262322-06A6-4CB7-AABC-78DA60730E07}"
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "tests", "tests", "{51B9C7D7-C306-485A-8EA9-38B32E928E95}"
EndProject
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "RandomDataGenerator.Tests", "..\tests\RandomDataGenerator.Tests\RandomDataGenerator.Tests.csproj", "{ED709720-2669-444E-AC90-ABB0B76A894A}"
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|Any CPU = Debug|Any CPU
Expand Down Expand Up @@ -77,6 +81,10 @@ Global
{13262322-06A6-4CB7-AABC-78DA60730E07}.Debug|Any CPU.Build.0 = Debug|Any CPU
{13262322-06A6-4CB7-AABC-78DA60730E07}.Release|Any CPU.ActiveCfg = Release|Any CPU
{13262322-06A6-4CB7-AABC-78DA60730E07}.Release|Any CPU.Build.0 = Release|Any CPU
{ED709720-2669-444E-AC90-ABB0B76A894A}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
{ED709720-2669-444E-AC90-ABB0B76A894A}.Debug|Any CPU.Build.0 = Debug|Any CPU
{ED709720-2669-444E-AC90-ABB0B76A894A}.Release|Any CPU.ActiveCfg = Release|Any CPU
{ED709720-2669-444E-AC90-ABB0B76A894A}.Release|Any CPU.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
Expand All @@ -90,6 +98,7 @@ Global
{94DA986E-B36A-4A9A-8B1E-F5BBAE82BEE7} = {CD43A6FA-4DEF-47B3-A430-9E7FFAD6B035}
{89571CAF-8CA9-44C2-98AB-D476CA2458DF} = {CD43A6FA-4DEF-47B3-A430-9E7FFAD6B035}
{13262322-06A6-4CB7-AABC-78DA60730E07} = {F8306255-6F4A-4E70-9932-06B2A3C9DF78}
{ED709720-2669-444E-AC90-ABB0B76A894A} = {51B9C7D7-C306-485A-8EA9-38B32E928E95}
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {F00DE8F4-BF0D-49C3-8854-600E8142BE41}
Expand Down
90 changes: 65 additions & 25 deletions src/RandomDataGenerator/Generators/RandomValueGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ namespace RandomDataGenerator.Generators
internal class RandomValueGenerator
{
private const double Tolerance = double.Epsilon;
private Random _rnf = new();
static Random _rnf = new();
static readonly object RandomLock = new object();
private double _storedUniformDeviate;
private bool _storedUniformDeviateIsGood;

Expand All @@ -27,7 +28,10 @@ public RandomValueGenerator(int seed)

public void Reset(int seed)
{
_rnf = new Random(seed);
lock (RandomLock)
{
_rnf = new Random(seed);
}
}
#endregion

Expand All @@ -38,23 +42,32 @@ public void Reset(int seed)
/// </summary>
public double Next()
{
return _rnf.NextDouble();
lock (RandomLock)
{
return _rnf.NextDouble();
}
}

/// <summary>
/// Returns true or false randomly.
/// </summary>
public bool NextBoolean()
{
return _rnf.Next(0, 2) != 0;
lock (RandomLock)
{
return _rnf.Next(0, 2) != 0;
}
}

/// <summary>
/// Returns double in the range [0, 1)
/// </summary>
public double NextDouble()
{
return _rnf.NextDouble();
lock (RandomLock)
{
return _rnf.NextDouble();
}
}

/// <summary>
Expand All @@ -68,7 +81,10 @@ public byte[] NextBytes(int min, int max)
int arrayLength = Next(min, max);

byte[] bytes = new byte[arrayLength];
_rnf.NextBytes(bytes);
lock (RandomLock)
{
_rnf.NextBytes(bytes);
}

return bytes;
}
Expand Down Expand Up @@ -140,8 +156,11 @@ public byte Next(byte min, byte max)
throw new ArgumentException("Max must be greater than min.");
}

double rn = (max * 1.0 - min * 1.0) * _rnf.NextDouble() + min * 1.0;
return Convert.ToByte(rn);
lock (RandomLock)
{
double rn = (max * 1.0 - min * 1.0) * _rnf.NextDouble() + min * 1.0;
return Convert.ToByte(rn);
}
}

/// <summary>
Expand All @@ -154,16 +173,22 @@ public short Next(short min, short max)
throw new ArgumentException("Max must be greater than min.");
}

double rn = (max * 1.0 - min * 1.0) * _rnf.NextDouble() + min * 1.0;
return Convert.ToInt16(rn);
lock (RandomLock)
{
double rn = (max * 1.0 - min * 1.0) * _rnf.NextDouble() + min * 1.0;
return Convert.ToInt16(rn);
}
}

/// <summary>
/// Returns Int32 in the range [min, max)
/// </summary>
public int Next(int min, int max)
{
return _rnf.Next(min, max);
lock (RandomLock)
{
return _rnf.Next(min, max);
}
}

/// <summary>
Expand All @@ -176,8 +201,11 @@ public long Next(long min, long max)
throw new ArgumentException("Max must be greater than min.");
}

double rn = (max * 1.0 - min * 1.0) * _rnf.NextDouble() + min * 1.0;
return Convert.ToInt64(rn);
lock (RandomLock)
{
double rn = (max * 1.0 - min * 1.0) * _rnf.NextDouble() + min * 1.0;
return Convert.ToInt64(rn);
}
}

/// <summary>
Expand All @@ -190,8 +218,11 @@ public float Next(float min, float max)
throw new ArgumentException("Max must be greater than min.");
}

double rn = (max * 1.0 - min * 1.0) * _rnf.NextDouble() + min * 1.0;
return Convert.ToSingle(rn);
lock (RandomLock)
{
double rn = (max * 1.0 - min * 1.0) * _rnf.NextDouble() + min * 1.0;
return Convert.ToSingle(rn);
}
}

/// <summary>
Expand All @@ -204,8 +235,11 @@ public double Next(double min, double max)
throw new ArgumentException("Max must be greater than min.");
}

double rn = (max - min) * _rnf.NextDouble() + min;
return rn;
lock (RandomLock)
{
double rn = (max - min) * _rnf.NextDouble() + min;
return rn;
}
}

/// <summary>
Expand All @@ -220,10 +254,13 @@ public DateTime Next(DateTime min, DateTime max)

long minTicks = min.Ticks;
long maxTicks = max.Ticks;
double rn = (Convert.ToDouble(maxTicks)
- Convert.ToDouble(minTicks)) * _rnf.NextDouble()
+ Convert.ToDouble(minTicks);
return new DateTime(Convert.ToInt64(rn));
lock (RandomLock)
{
double rn = (Convert.ToDouble(maxTicks)
- Convert.ToDouble(minTicks)) * _rnf.NextDouble()
+ Convert.ToDouble(minTicks);
return new DateTime(Convert.ToInt64(rn));
}
}

/// <summary>
Expand All @@ -238,10 +275,13 @@ public TimeSpan Next(TimeSpan min, TimeSpan max)

long minTicks = min.Ticks;
long maxTicks = max.Ticks;
double rn = (Convert.ToDouble(maxTicks)
- Convert.ToDouble(minTicks)) * _rnf.NextDouble()
+ Convert.ToDouble(minTicks);
return new TimeSpan(Convert.ToInt64(rn));
lock (RandomLock)
{
double rn = (Convert.ToDouble(maxTicks)
- Convert.ToDouble(minTicks)) * _rnf.NextDouble()
+ Convert.ToDouble(minTicks);
return new TimeSpan(Convert.ToInt64(rn));
}
}

/// <summary>
Expand Down
81 changes: 81 additions & 0 deletions tests/RandomDataGenerator.Tests/CityRandomizerTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
using System.Collections.Concurrent;
using RandomDataGenerator.FieldOptions;
using RandomDataGenerator.Randomizers;
using Xunit.Abstractions;

namespace RandomDataGenerator.Tests
{
public class CityRandomizerTests
{
private readonly ITestOutputHelper _output;
static readonly Random random = new System.Random(420);
static readonly object randLock = new object();

public CityRandomizerTests(ITestOutputHelper output)
{
this._output = output;
}


[Theory]
[InlineData(1)]
[InlineData(2)]
[InlineData(4)]
[InlineData(8)]
public void CityDistributionMustBeUniform(int degree)
{
var locationGenerator = RandomizerFactory.GetRandomizer(new FieldOptionsCity { ValueAsString = true, UseNullValues = false });
var concurrentDictionary = new ConcurrentDictionary<string, long>();
var options = new ParallelOptions { MaxDegreeOfParallelism = degree };
Parallel.For(0, 1000, options, i =>
{
Parallel.For(0, 1000, options, j =>
{
var location = locationGenerator.Generate();
concurrentDictionary.AddOrUpdate(location, _ => 1, (k, v) => v + 1);
});
});
var topCount = concurrentDictionary.OrderByDescending(pair => pair.Value).First();
var bottomCount = concurrentDictionary.OrderBy(pair => pair.Value).First();
_output.WriteLine($"{topCount}");
_output.WriteLine($"{bottomCount}");
Assert.True(topCount.Value/bottomCount.Value<2);
Assert.NotEqual(topCount.Key, bottomCount.Key);
}

[Fact]
public void TwoRandomCitiesMustNotBeTheSame()
{
var locationGenerator = RandomizerFactory.GetRandomizer(new FieldOptionsCity { ValueAsString = true, UseNullValues = false });
var locationOne = locationGenerator.Generate();
var locationTwo = locationGenerator.Generate();
Assert.NotEqual(locationOne, locationTwo);
}

[Fact]
public void SystemRandomDistributionMustBeUniform()
{
var concurrentDictionary = new ConcurrentDictionary<int, long>();
Parallel.For(0, 1000, i =>
{
Parallel.For(0, 1000, j =>
{
int index;
lock (randLock)
{
index = random.Next(0, 2000);
}
concurrentDictionary.AddOrUpdate(index, _ => 1, (k, v) => v + 1);
});
});
var topCount = concurrentDictionary.OrderByDescending(pair => pair.Value).First();
var bottomCount = concurrentDictionary.OrderBy(pair => pair.Value).First();
_output.WriteLine($"{topCount}");
_output.WriteLine($"{bottomCount}");
Assert.True(topCount.Value / bottomCount.Value < 2);
Assert.NotEqual(topCount.Key, bottomCount.Key);

}
}
}
28 changes: 28 additions & 0 deletions tests/RandomDataGenerator.Tests/RandomDataGenerator.Tests.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net7.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>

<IsPackable>false</IsPackable>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
<PackageReference Include="xunit" Version="2.4.2" />
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.5">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
<PackageReference Include="coverlet.collector" Version="3.1.2">
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
</PackageReference>
</ItemGroup>

<ItemGroup>
<ProjectReference Include="..\..\src\RandomDataGenerator\RandomDataGenerator.csproj" />
</ItemGroup>

</Project>
1 change: 1 addition & 0 deletions tests/RandomDataGenerator.Tests/Usings.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
global using Xunit;

0 comments on commit 653818f

Please sign in to comment.