Skip to content

Commit

Permalink
Random number generator is not thread safe (dotnet#310)
Browse files Browse the repository at this point in the history
* Random number generator is not thread safe

* Another local random generator

* Missed a few references

* Referncing AutoMlUtils.random instead of a local RNG

* More refs to mail RNG; remove Float as per dotnet#1669

* Missed Random.cs
  • Loading branch information
justinormont authored Mar 26, 2019
1 parent 35e5bbc commit e152288
Show file tree
Hide file tree
Showing 10 changed files with 43 additions and 45 deletions.
1 change: 1 addition & 0 deletions src/Microsoft.ML.Auto/API/ExperimentSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public class ExperimentSettings
/// </summary>
public DirectoryInfo ModelDirectory { get; set; } = null;

/// <summary>
/// This setting controls whether or not an AutoML experiment will make use of ML.NET-provided caching.
/// If set to true, caching will be forced on for all pipelines. If set to false, caching will be forced off.
/// If set to null (default value), AutoML will decide whether to enable caching for each model.
Expand Down
5 changes: 3 additions & 2 deletions src/Microsoft.ML.Auto/AutoMlUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,18 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Threading;
using Microsoft.Data.DataView;

namespace Microsoft.ML.Auto
{
internal static class AutoMlUtils
{
public static Random Random = new Random();
public static readonly ThreadLocal<Random> random = new ThreadLocal<Random>(() => new Random());

public static void Assert(bool boolVal, string message = null)
{
if(!boolVal)
if (!boolVal)
{
message = message ?? "Assertion failed";
throw new InvalidOperationException(message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public void Apply(IntermediateColumn[] columns)
if (!col.RawData.Skip(1)
.All(x =>
{
Single value;
float value;
return Conversions.TryParse(in x, out value);
})
)
Expand Down
3 changes: 1 addition & 2 deletions src/Microsoft.ML.Auto/ColumnInference/TextFileSample.cs
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,8 @@ public static TextFileSample CreateFromFullStream(Stream stream)
// determine the start of each remaining chunk
long fileSizeRemaining = fileSize - firstChunk.Length - ((long)chunkSize) * chunkCount;

var rnd = AutoMlUtils.Random;
var chunkStartIndices = Enumerable.Range(0, chunkCount)
.Select(x => rnd.NextDouble() * fileSizeRemaining)
.Select(x => AutoMlUtils.random.Value.NextDouble() * fileSizeRemaining)
.OrderBy(x => x)
.Select((spot, i) => (long)(spot + firstChunk.Length + i * chunkSize))
.ToArray();
Expand Down
13 changes: 6 additions & 7 deletions src/Microsoft.ML.Auto/Sweepers/ISweeper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
using System.Collections;
using System.Collections.Generic;
using System.Linq;
using Float = System.Single;

namespace Microsoft.ML.Auto
{
Expand Down Expand Up @@ -236,10 +235,10 @@ IComparable IRunResult.MetricValue
/// </summary>
internal sealed class RunMetric
{
private readonly Float _primaryMetric;
private readonly Float[] _metricDistribution;
private readonly float _primaryMetric;
private readonly float[] _metricDistribution;

public RunMetric(Float primaryMetric, IEnumerable<Float> metricDistribution = null)
public RunMetric(float primaryMetric, IEnumerable<float> metricDistribution = null)
{
_primaryMetric = primaryMetric;
if (metricDistribution != null)
Expand All @@ -252,7 +251,7 @@ public RunMetric(Float primaryMetric, IEnumerable<Float> metricDistribution = nu
/// By default, smart sweeping algorithms will maximize this metric.
/// If you want to minimize, either negate this value or change the option in the arguments of the sweeper constructor.
/// </summary>
public Float PrimaryMetric
public float PrimaryMetric
{
get { return _primaryMetric; }
}
Expand All @@ -261,11 +260,11 @@ public Float PrimaryMetric
/// The (optional) distribution of the metric.
/// This distribution can be a secondary measure of how good a run was, e.g per-fold AUC, per-fold accuracy, (sampled) per-instance log loss etc.
/// </summary>
public Float[] GetMetricDistribution()
public float[] GetMetricDistribution()
{
if (_metricDistribution == null)
return null;
var result = new Float[_metricDistribution.Length];
var result = new float[_metricDistribution.Length];
Array.Copy(_metricDistribution, result, _metricDistribution.Length);
return result;
}
Expand Down
45 changes: 22 additions & 23 deletions src/Microsoft.ML.Auto/Sweepers/Parameters.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

using System;
using System.Collections.Generic;
using Float = System.Single;

namespace Microsoft.ML.Auto
{
Expand All @@ -29,10 +28,10 @@ internal abstract class NumericParamArguments : BaseParamArguments
internal class FloatParamArguments : NumericParamArguments
{
//[Argument(ArgumentType.Required, HelpText = "Minimum value")]
public Float Min;
public float Min;

//[Argument(ArgumentType.Required, HelpText = "Maximum value")]
public Float Max;
public float Max;
}

internal class LongParamArguments : NumericParamArguments
Expand Down Expand Up @@ -95,11 +94,11 @@ public override int GetHashCode()
}
}

internal sealed class FloatParameterValue : IParameterValue<Float>
internal sealed class FloatParameterValue : IParameterValue<float>
{
private readonly string _name;
private readonly string _valueText;
private readonly Float _value;
private readonly float _value;

public string Name
{
Expand All @@ -111,14 +110,14 @@ public string ValueText
get { return _valueText; }
}

public Float Value
public float Value
{
get { return _value; }
}

public FloatParameterValue(string name, Float value)
public FloatParameterValue(string name, float value)
{
AutoMlUtils.Assert(!Float.IsNaN(value));
AutoMlUtils.Assert(!float.IsNaN(value));
_name = name;
_value = value;
_valueText = _value.ToString("R");
Expand Down Expand Up @@ -186,7 +185,7 @@ public override int GetHashCode()

internal interface INumericValueGenerator : IValueGenerator
{
Float NormalizeValue(IParameterValue value);
float NormalizeValue(IParameterValue value);
bool InRange(IParameterValue value);
}

Expand Down Expand Up @@ -294,19 +293,19 @@ public int Count
}
}

public Float NormalizeValue(IParameterValue value)
public float NormalizeValue(IParameterValue value)
{
var valueTyped = value as LongParameterValue;
AutoMlUtils.Assert(valueTyped != null, "LongValueGenerator could not normalized parameter because it is not of the correct type");
AutoMlUtils.Assert(_args.Min <= valueTyped.Value && valueTyped.Value <= _args.Max, "Value not in correct range");

if (_args.LogBase)
{
Float logBase = (Float)(_args.StepSize ?? Math.Pow(1.0 * _args.Max / _args.Min, 1.0 / (_args.NumSteps - 1)));
return (Float)((Math.Log(valueTyped.Value, logBase) - Math.Log(_args.Min, logBase)) / (Math.Log(_args.Max, logBase) - Math.Log(_args.Min, logBase)));
float logBase = (float)(_args.StepSize ?? Math.Pow(1.0 * _args.Max / _args.Min, 1.0 / (_args.NumSteps - 1)));
return (float)((Math.Log(valueTyped.Value, logBase) - Math.Log(_args.Min, logBase)) / (Math.Log(_args.Max, logBase) - Math.Log(_args.Min, logBase)));
}
else
return (Float)(valueTyped.Value - _args.Min) / (_args.Max - _args.Min);
return (float)(valueTyped.Value - _args.Min) / (_args.Max - _args.Min);
}

public bool InRange(IParameterValue value)
Expand Down Expand Up @@ -339,7 +338,7 @@ public FloatValueGenerator(FloatParamArguments args)
// REVIEW: Is Float accurate enough?
public IParameterValue CreateFromNormalized(Double normalizedValue)
{
Float val;
float val;
if (_args.LogBase)
{
// REVIEW: review the math below, it only works for positive Min and Max
Expand All @@ -348,10 +347,10 @@ public IParameterValue CreateFromNormalized(Double normalizedValue)
: _args.StepSize.Value;
var logMax = Math.Log(_args.Max, logBase);
var logMin = Math.Log(_args.Min, logBase);
val = (Float)(_args.Min * Math.Pow(logBase, normalizedValue * (logMax - logMin)));
val = (float)(_args.Min * Math.Pow(logBase, normalizedValue * (logMax - logMin)));
}
else
val = (Float)(_args.Min + normalizedValue * (_args.Max - _args.Min));
val = (float)(_args.Min + normalizedValue * (_args.Max - _args.Min));

return new FloatParameterValue(_args.Name, val);
}
Expand All @@ -367,11 +366,11 @@ private void EnsureParameterValues()
// REVIEW: review the math below, it only works for positive Min and Max
var logBase = _args.StepSize ?? Math.Pow(1.0 * _args.Max / _args.Min, 1.0 / (_args.NumSteps - 1));

Float prevValue = Float.NegativeInfinity;
float prevValue = float.NegativeInfinity;
var maxPlusEpsilon = _args.Max * Math.Sqrt(logBase);
for (Double value = _args.Min; value <= maxPlusEpsilon; value *= logBase)
{
var floatValue = (Float)value;
var floatValue = (float)value;
if (floatValue > prevValue)
result.Add(new FloatParameterValue(_args.Name, floatValue));
prevValue = floatValue;
Expand All @@ -380,11 +379,11 @@ private void EnsureParameterValues()
else
{
var stepSize = _args.StepSize ?? (Double)(_args.Max - _args.Min) / (_args.NumSteps - 1);
Float prevValue = Float.NegativeInfinity;
float prevValue = float.NegativeInfinity;
var maxPlusEpsilon = _args.Max + stepSize / 2;
for (Double value = _args.Min; value <= maxPlusEpsilon; value += stepSize)
{
var floatValue = (Float)value;
var floatValue = (float)value;
if (floatValue > prevValue)
result.Add(new FloatParameterValue(_args.Name, floatValue));
prevValue = floatValue;
Expand Down Expand Up @@ -412,16 +411,16 @@ public int Count
}
}

public Float NormalizeValue(IParameterValue value)
public float NormalizeValue(IParameterValue value)
{
var valueTyped = value as FloatParameterValue;
AutoMlUtils.Assert(valueTyped != null, "FloatValueGenerator could not normalized parameter because it is not of the correct type");
AutoMlUtils.Assert(_args.Min <= valueTyped.Value && valueTyped.Value <= _args.Max, "Value not in correct range");

if (_args.LogBase)
{
Float logBase = (Float)(_args.StepSize ?? Math.Pow(1.0 * _args.Max / _args.Min, 1.0 / (_args.NumSteps - 1)));
return (Float)((Math.Log(valueTyped.Value, logBase) - Math.Log(_args.Min, logBase)) / (Math.Log(_args.Max, logBase) - Math.Log(_args.Min, logBase)));
float logBase = (float)(_args.StepSize ?? Math.Pow(1.0 * _args.Max / _args.Min, 1.0 / (_args.NumSteps - 1)));
return (float)((Math.Log(valueTyped.Value, logBase) - Math.Log(_args.Min, logBase)) / (Math.Log(_args.Max, logBase) - Math.Log(_args.Min, logBase)));
}
else
return (valueTyped.Value - _args.Min) / (_args.Max - _args.Min);
Expand Down
2 changes: 1 addition & 1 deletion src/Microsoft.ML.Auto/Sweepers/Random.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public UniformRandomSweeper(ArgumentsBase args, IValueGenerator[] sweepParameter

protected override ParameterSet CreateParamSet()
{
return new ParameterSet(SweepParameters.Select(sweepParameter => sweepParameter.CreateFromNormalized(AutoMlUtils.Random.NextDouble())));
return new ParameterSet(SweepParameters.Select(sweepParameter => sweepParameter.CreateFromNormalized(AutoMlUtils.random.Value.NextDouble())));
}
}
}
11 changes: 5 additions & 6 deletions src/Microsoft.ML.Auto/Sweepers/SweeperProbabilityUtils.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

using System;
using System.Collections.Generic;
using Float = System.Single;

namespace Microsoft.ML.Auto
{
Expand Down Expand Up @@ -35,8 +34,8 @@ public double[] NormalRVs(int numRVs, double mu, double sigma)

for (int i = 0; i < numRVs; i++)
{
u1 = AutoMlUtils.Random.NextDouble();
u2 = AutoMlUtils.Random.NextDouble();
u1 = AutoMlUtils.random.Value.NextDouble();
u2 = AutoMlUtils.random.Value.NextDouble();
rvs.Add(mu + sigma * Math.Sqrt(-2.0 * Math.Log(u1)) * Math.Sin(2.0 * Math.PI * u2));
}

Expand All @@ -61,11 +60,11 @@ private int BinarySearch(double[] a, double u, int low, int high)
return a[mid] >= u ? BinarySearch(a, u, low, mid) : BinarySearch(a, u, mid, high);
}

public static Float[] ParameterSetAsFloatArray(IValueGenerator[] sweepParams, ParameterSet ps, bool expandCategoricals = true)
public static float[] ParameterSetAsFloatArray(IValueGenerator[] sweepParams, ParameterSet ps, bool expandCategoricals = true)
{
AutoMlUtils.Assert(ps.Count == sweepParams.Length);

var result = new List<Float>();
var result = new List<float>();

for (int i = 0; i < sweepParams.Length; i++)
{
Expand Down Expand Up @@ -115,7 +114,7 @@ public static Float[] ParameterSetAsFloatArray(IValueGenerator[] sweepParams, Pa
return result.ToArray();
}

public static ParameterSet FloatArrayAsParameterSet(IValueGenerator[] sweepParams, Float[] array, bool expandedCategoricals = true)
public static ParameterSet FloatArrayAsParameterSet(IValueGenerator[] sweepParams, float[] array, bool expandedCategoricals = true)
{
AutoMlUtils.Assert(array.Length == sweepParams.Length);

Expand Down
4 changes: 2 additions & 2 deletions src/Test/GetNextPipelineTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ public void GetNextPipelineMock()
break;
}

var result = new PipelineScore(pipeline, AutoMlUtils.Random.NextDouble(), true);
var result = new PipelineScore(pipeline, AutoMlUtils.random.Value.NextDouble(), true);
history.Add(result);
}

Assert.AreEqual(maxIterations, history.Count);

// Get all 'Stage 1' and 'Stage 2' runs from Pipeline Suggester
Expand Down
2 changes: 1 addition & 1 deletion src/Test/TextFileSampleTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public void CanParseLargeRandomStream()
for (var i = 0; i < numRows; i++)
{
var row = new byte[rowSize];
AutoMlUtils.Random.NextBytes(row);
AutoMlUtils.random.Value.NextBytes(row);

// ensure byte array has no 0s, so text file sampler doesn't
// think file is encoded with UTF-16 or UTF-32 without a BOM
Expand Down

0 comments on commit e152288

Please sign in to comment.