From 20e6a408d668d46c8dd7a294a462c6935e087c4d Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Fri, 14 Aug 2020 11:13:43 -0500 Subject: [PATCH 1/4] Added unit test class Issue319 for #319 with test cases decimal_with_very_large_range_succeeds to Bogus.Tests. Updated Randomizer.Decimal to make the test pass. --- Source/Bogus.Tests/GitHubIssues/Issue319.cs | 48 +++++++++++++++++++ Source/Bogus/Randomizer.cs | 52 ++++++++++++++++++++- 2 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 Source/Bogus.Tests/GitHubIssues/Issue319.cs diff --git a/Source/Bogus.Tests/GitHubIssues/Issue319.cs b/Source/Bogus.Tests/GitHubIssues/Issue319.cs new file mode 100644 index 00000000..e502522e --- /dev/null +++ b/Source/Bogus.Tests/GitHubIssues/Issue319.cs @@ -0,0 +1,48 @@ +using System.Collections.Generic; +using System.Reflection; +using FluentAssertions; +using Xunit; +using Xunit.Abstractions; +using Xunit.Sdk; + +namespace Bogus.Tests.GitHubIssues +{ + public class Issue319 : SeededTest + { + class TestDataProvider : DataAttribute + { + public override IEnumerable GetData(MethodInfo testMethod) + { + yield return new object[] { 0m, decimal.MaxValue }; + yield return new object[] { decimal.MinValue, decimal.MaxValue }; + yield return new object[] { decimal.MinValue, 0m }; + // A range whose size exceeds decimal.MaxValue but which doesn't have decimal.MinValue or decimal.MaxValue as a bound. + yield return new object[] { decimal.MinValue * 0.6m, decimal.MaxValue * 0.6m }; + } + } + + ITestOutputHelper _output; + + public Issue319(ITestOutputHelper output) + => _output = output; + + [Theory, TestDataProvider] + public void decimal_with_very_large_range_succeeds(decimal min, decimal max) + { + var randomizer = new Randomizer(); + + for (int iteration = 0; iteration < 300; iteration++) + { + try + { + randomizer.Decimal(min, max).Should().BeInRange(min, max); + } + catch + { + _output.WriteLine("Test failed on iteration {0}", iteration); + throw; + } + } + } + } +} diff --git a/Source/Bogus/Randomizer.cs b/Source/Bogus/Randomizer.cs index dc85796e..66ca43b2 100644 --- a/Source/Bogus/Randomizer.cs +++ b/Source/Bogus/Randomizer.cs @@ -194,6 +194,14 @@ public double Double(double min = 0.0d, double max = 1.0d) /// Maximum, default 1.0 public decimal Decimal(decimal min = 0.0m, decimal max = 1.0m) { + if (min > max) + { + decimal tmp = min; + + min = max; + max = tmp; + } + // Decimal: 128 bits wide // bit 0: sign bit // bit 1-10: not used @@ -221,10 +229,50 @@ public decimal Decimal(decimal min = 0.0m, decimal max = 1.0m) decimal result = new decimal(lowBits, middleBits, highBits, isNegative: false, Scale); - // Step 2: Scale the value and adjust it to the desired range. This may decrease + // Step 2: Figure out how much of the scale we can keep without causing an overflow. + // Note that the range can actually exceed decimal.MaxValue, e.g. if max is itself + // decimal.MaxValue and min is negative. So, we work with half the range, using this + // scale factor that is as close to 0.5 as possible without causing the result of + // decimal.MaxValue * ScaleFactor to round up. If it rounds up, then the result of + // decimal.MaxValue * ScaleFactor - decimal.MinValue * ScaleFactor will still be + // larger than decimal.MaxValue. + const decimal OneHalfScaleFactor = 0.4999999999999999999999999999m; + + decimal halfRange = max * OneHalfScaleFactor - min * OneHalfScaleFactor; + + // Two reasons we're forced to use a scaled multiplier: + // + // 1. The range (max - min) is itself too large to store in decimal.MaxValue. + // 2. The result of result * (max - min) is too large to store in decimal.MaxValue. + // + // Check condition 1: + bool useScaledMultiplier = (halfRange >= decimal.MaxValue * OneHalfScaleFactor); + + decimal multiplier = halfRange; + decimal divisor = 3.9614081257132168796771975168m; + + // Check condition 2: + if (result >= 1.0m) + { + decimal maximumMultiplier = decimal.MaxValue / result; + + while (multiplier >= maximumMultiplier) + { + // Drop one digit of precision and try again. + multiplier *= 0.1m; + divisor *= 10m; + + useScaledMultiplier = true; + } + } + + // Step 3: Scale the value and adjust it to the desired range. This may decrease // the accuracy by adjusting the scale as necessary, but we get the best possible // outcome by starting with the most precise scale. - return result * (max - min) / 7.9228162514264337593543950335m + min; + if (useScaledMultiplier) + return result * multiplier / divisor + min; + else + return result * (max - min) / 7.9228162514264337593543950335m + min; } /// From a5c446a78c77fa3e19b6b96aa691b949c19784ec Mon Sep 17 00:00:00 2001 From: Jonathan Gilbert Date: Fri, 14 Aug 2020 12:48:20 -0500 Subject: [PATCH 2/4] Refactored Randomizer.Decimal to allow better unit testing into separate methods GenerateRandomMaximumPrecisionDecimal and ScaleMaximumPrecisionDecimalToRange. Added unit tests to RandomizerTest.cs. Tweaked the implementation of GenerateRandomMaximumPrecisionDecimal and ScaleMaximumPrecisionDecimalToRange based on observations driven by the tests. --- Source/Bogus.Tests/RandomizerTest.cs | 50 ++++++++++++- Source/Bogus/Randomizer.cs | 107 ++++++++++++++++++++------- 2 files changed, 128 insertions(+), 29 deletions(-) diff --git a/Source/Bogus.Tests/RandomizerTest.cs b/Source/Bogus.Tests/RandomizerTest.cs index b9394ebc..132a6cab 100644 --- a/Source/Bogus.Tests/RandomizerTest.cs +++ b/Source/Bogus.Tests/RandomizerTest.cs @@ -1,10 +1,12 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Reflection; using System.Text; using FluentAssertions; using Xunit; using Xunit.Abstractions; +using Xunit.Sdk; namespace Bogus.Tests { @@ -194,7 +196,53 @@ public void generate_double_with_min_and_max() [Fact] public void generate_decimal_with_min_and_max() { - r.Decimal(2.2m, 5.2m).Should().Be(3.8697355489728032005903907232m); + r.Decimal(2.2m, 5.2m).Should().Be(2.8651322255135983997048046384m); + } + + [Fact] + public void generate_maximum_range_decimal() + { + const decimal SmallestMaxPrecision = -7.9228162514264337593543950335m; + const decimal LargestMaxPrecision = 7.9228162514264337593543950335m; + + for (int iteration = 0; iteration < 1000; iteration++) + { + try + { + r.GenerateRandomMaximumPrecisionDecimal().Should().BeInRange(SmallestMaxPrecision, LargestMaxPrecision); + } + catch + { + console.WriteLine("Test failed on iteration {0}", iteration); + throw; + } + } + } + + class DecimalScaleDataProvider : DataAttribute + { + public override IEnumerable GetData(MethodInfo testMethod) + { + yield return new object[] { 0m, 1m, 0.5m }; + yield return new object[] { -100m, 100m, 0m }; + yield return new object[] { decimal.MinValue, decimal.MaxValue, 0m }; + yield return new object[] { 0m, decimal.MaxValue, decimal.MaxValue * 0.5m - 1m }; + yield return new object[] { decimal.MinValue, 0m, decimal.MinValue * 0.5m + 1m }; + yield return new object[] { 0m, 0m, 0m }; + yield return new object[] { decimal.MinValue, decimal.MinValue, decimal.MinValue }; + yield return new object[] { decimal.MaxValue, decimal.MaxValue, decimal.MaxValue }; + } + } + + [Theory, DecimalScaleDataProvider] + public void scale_decimal_with_min_and_max(decimal min, decimal max, decimal middle) + { + const decimal SmallestMaxPrecision = -7.9228162514264337593543950335m; + const decimal LargestMaxPrecision = 7.9228162514264337593543950335m; + + r.ScaleMaximumPrecisionDecimalToRange(SmallestMaxPrecision, min, max).Should().Be(min); + r.ScaleMaximumPrecisionDecimalToRange(LargestMaxPrecision, min, max).Should().Be(max); + r.ScaleMaximumPrecisionDecimalToRange(0m, min, max).Should().Be(middle); } [Fact] diff --git a/Source/Bogus/Randomizer.cs b/Source/Bogus/Randomizer.cs index 66ca43b2..98f88a52 100644 --- a/Source/Bogus/Randomizer.cs +++ b/Source/Bogus/Randomizer.cs @@ -188,20 +188,12 @@ public double Double(double min = 0.0d, double max = 1.0d) } /// - /// Get a random decimal, between 0.0 and 1.0. + /// Generates a decimal with maximum precision with uniform distribution across the range of + /// decimal values with maximum scaling: (decimal.MinValue .. decimal.MaxValue) divided by 10^28. /// - /// Minimum, default 0.0 - /// Maximum, default 1.0 - public decimal Decimal(decimal min = 0.0m, decimal max = 1.0m) + /// + internal decimal GenerateRandomMaximumPrecisionDecimal() { - if (min > max) - { - decimal tmp = min; - - min = max; - max = tmp; - } - // Decimal: 128 bits wide // bit 0: sign bit // bit 1-10: not used @@ -215,21 +207,36 @@ public decimal Decimal(decimal min = 0.0m, decimal max = 1.0m) // Max value with max scaling: 001C0000 FFFFFFFF FFFFFFFF FFFFFFFF // = 7.9228162514264337593543950335 - // Step 1: Generate a value with uniform distribution between 0 and this value. - // This ensures the greatest level of precision in the distribution of bits; - // the resulting value, after it is adjusted into the caller's desired range, - // should not skip any possible values at the least significant end of the - // mantissa. - int lowBits = Number(int.MinValue, int.MaxValue); int middleBits = Number(int.MinValue, int.MaxValue); int highBits = Number(int.MinValue, int.MaxValue); + bool isNegative = Bool(); + const int Scale = 28; - decimal result = new decimal(lowBits, middleBits, highBits, isNegative: false, Scale); + return new decimal(lowBits, middleBits, highBits, isNegative, Scale); + } + + /// + /// Takes a decimal in the range in which decimal has maximum precision ((decimal.MinValue .. decimal.MaxValue) scaled by the maximum + /// scaling factor of 10^28) and transforms it into the caller's desired range, reducing precision only as much as needed. + /// + /// A decimal in the range (decimal.MinValue / SCALE .. decimal.MaxValue / SCALE), where SCALE is the maximum scaling factor (10^28). + /// The lower bound of the target range. + /// The upper bound of the target range. + /// + internal decimal ScaleMaximumPrecisionDecimalToRange(decimal input, decimal min, decimal max) + { + const decimal SmallestMaxPrecision = -7.9228162514264337593543950335m; + const decimal LargestMaxPrecision = 7.9228162514264337593543950335m; + + if (input <= SmallestMaxPrecision) + return min; + if (input >= LargestMaxPrecision) + return max; - // Step 2: Figure out how much of the scale we can keep without causing an overflow. + // Step 1: Figure out how much of the scale we can keep without causing an overflow. // Note that the range can actually exceed decimal.MaxValue, e.g. if max is itself // decimal.MaxValue and min is negative. So, we work with half the range, using this // scale factor that is as close to 0.5 as possible without causing the result of @@ -246,33 +253,77 @@ public decimal Decimal(decimal min = 0.0m, decimal max = 1.0m) // 2. The result of result * (max - min) is too large to store in decimal.MaxValue. // // Check condition 1: - bool useScaledMultiplier = (halfRange >= decimal.MaxValue * OneHalfScaleFactor); + bool useScaledMultiplier = (halfRange > decimal.MaxValue * OneHalfScaleFactor); decimal multiplier = halfRange; - decimal divisor = 3.9614081257132168796771975168m; + decimal divisor = 7.922816251426433759354395032m; // This value is the maximum value at maximum precision times (OneHalfScaleFactor / 0.5). // Check condition 2: - if (result >= 1.0m) + decimal inputMagnitude = Math.Abs(input); + + if (inputMagnitude >= 1.0m) { - decimal maximumMultiplier = decimal.MaxValue / result; + decimal maximumMultiplier = decimal.MaxValue / inputMagnitude; while (multiplier >= maximumMultiplier) { // Drop one digit of precision and try again. multiplier *= 0.1m; - divisor *= 10m; + divisor *= 0.1m; useScaledMultiplier = true; } } - // Step 3: Scale the value and adjust it to the desired range. This may decrease + // Step 2: Scale the value and adjust it to the desired range. This may decrease // the accuracy by adjusting the scale as necessary, but we get the best possible // outcome by starting with the most precise scale. if (useScaledMultiplier) - return result * multiplier / divisor + min; + { + decimal rangeMiddle = max * OneHalfScaleFactor + min * OneHalfScaleFactor; + + return rangeMiddle + input * multiplier / divisor; + } else - return result * (max - min) / 7.9228162514264337593543950335m + min; + { + // If we're dealing with values at the upper extreme ends (decimal.MinValue, decimal.MaxValue), + // then half their value can't be represented in a decimal and will be subject to rounding. The + // result of that rounding is that adding the values together will produce an out-of-range value. + // Therefore, we can't use a straightforward (min + max) * 0.5, or even (min * 0.5 + max * 0.5). + decimal rangeSize = max - min; + decimal rangeMiddle; + + if (min == max) + rangeMiddle = min; + else if (-min == max) + rangeMiddle = 0m; + else if (Math.Abs(min) < Math.Abs(max)) + rangeMiddle = max - rangeSize * 0.5m; + else + rangeMiddle = min + rangeSize * 0.5m; + + return rangeMiddle + input * rangeSize / 15.845632502852867518708790067m; + } + } + + /// + /// Get a random decimal, between 0.0 and 1.0. + /// + /// Minimum, default 0.0 + /// Maximum, default 1.0 + public decimal Decimal(decimal min = 0.0m, decimal max = 1.0m) + { + if (min > max) + { + decimal tmp = min; + + min = max; + max = tmp; + } + + var unscaledSample = GenerateRandomMaximumPrecisionDecimal(); + + return ScaleMaximumPrecisionDecimalToRange(unscaledSample, min, max); } /// From 67691322105358527e615af2f483fa42252bbd3f Mon Sep 17 00:00:00 2001 From: bchavez Date: Sat, 15 Aug 2020 18:02:51 -0700 Subject: [PATCH 3/4] Moving PR to an extension method - to allow rebase. --- .../{Issue319.cs => Issue319_2.cs} | 4 +- .../Extensions/ExtensionsForRandomizer2.cs | 142 ++++++++++++++++++ Source/Bogus/Randomizer.cs | 131 +--------------- 3 files changed, 145 insertions(+), 132 deletions(-) rename Source/Bogus.Tests/GitHubIssues/{Issue319.cs => Issue319_2.cs} (93%) create mode 100644 Source/Bogus/Extensions/ExtensionsForRandomizer2.cs diff --git a/Source/Bogus.Tests/GitHubIssues/Issue319.cs b/Source/Bogus.Tests/GitHubIssues/Issue319_2.cs similarity index 93% rename from Source/Bogus.Tests/GitHubIssues/Issue319.cs rename to Source/Bogus.Tests/GitHubIssues/Issue319_2.cs index e502522e..e92ed5d8 100644 --- a/Source/Bogus.Tests/GitHubIssues/Issue319.cs +++ b/Source/Bogus.Tests/GitHubIssues/Issue319_2.cs @@ -7,7 +7,7 @@ namespace Bogus.Tests.GitHubIssues { - public class Issue319 : SeededTest + public class Issue319_2 : SeededTest { class TestDataProvider : DataAttribute { @@ -23,7 +23,7 @@ public override IEnumerable GetData(MethodInfo testMethod) ITestOutputHelper _output; - public Issue319(ITestOutputHelper output) + public Issue319_2(ITestOutputHelper output) => _output = output; [Theory, TestDataProvider] diff --git a/Source/Bogus/Extensions/ExtensionsForRandomizer2.cs b/Source/Bogus/Extensions/ExtensionsForRandomizer2.cs new file mode 100644 index 00000000..007edeba --- /dev/null +++ b/Source/Bogus/Extensions/ExtensionsForRandomizer2.cs @@ -0,0 +1,142 @@ +using System; + +namespace Bogus.Extensions +{ + public static class ExtensionsForRandomizer2 + { + public static decimal Decimal3(this Randomizer r, decimal min = 0.0m, decimal max = 1.0m) + { + if (min > max) + { + decimal tmp = min; + + min = max; + max = tmp; + } + + var unscaledSample = GenerateRandomMaximumPrecisionDecimal(r); + + return ScaleMaximumPrecisionDecimalToRange(unscaledSample, min, max); + } + + + /// + /// Generates a decimal with maximum precision with uniform distribution across the range of + /// decimal values with maximum scaling: (decimal.MinValue .. decimal.MaxValue) divided by 10^28. + /// + /// + internal static decimal GenerateRandomMaximumPrecisionDecimal(Randomizer r) + { + // Decimal: 128 bits wide + // bit 0: sign bit + // bit 1-10: not used + // bit 11-15: scale (values 29, 30, 31 not used) + // bit 16-31: not used + // bit 32-127: mantissa (96 bits) + + // Max value: 00000000 FFFFFFFF FFFFFFFF FFFFFFFF + // = 79228162514264337593543950335 + + // Max value with max scaling: 001C0000 FFFFFFFF FFFFFFFF FFFFFFFF + // = 7.9228162514264337593543950335 + + int lowBits = r.Number(int.MinValue, int.MaxValue); + int middleBits = r.Number(int.MinValue, int.MaxValue); + int highBits = r.Number(int.MinValue, int.MaxValue); + + bool isNegative = r.Bool(); + + const int Scale = 28; + + return new decimal(lowBits, middleBits, highBits, isNegative, Scale); + } + + /// + /// Takes a decimal in the range in which decimal has maximum precision ((decimal.MinValue .. decimal.MaxValue) scaled by the maximum + /// scaling factor of 10^28) and transforms it into the caller's desired range, reducing precision only as much as needed. + /// + /// A decimal in the range (decimal.MinValue / SCALE .. decimal.MaxValue / SCALE), where SCALE is the maximum scaling factor (10^28). + /// The lower bound of the target range. + /// The upper bound of the target range. + /// + internal static decimal ScaleMaximumPrecisionDecimalToRange(decimal input, decimal min, decimal max) + { + const decimal SmallestMaxPrecision = -7.9228162514264337593543950335m; + const decimal LargestMaxPrecision = 7.9228162514264337593543950335m; + + if (input <= SmallestMaxPrecision) + return min; + if (input >= LargestMaxPrecision) + return max; + + // Step 1: Figure out how much of the scale we can keep without causing an overflow. + // Note that the range can actually exceed decimal.MaxValue, e.g. if max is itself + // decimal.MaxValue and min is negative. So, we work with half the range, using this + // scale factor that is as close to 0.5 as possible without causing the result of + // decimal.MaxValue * ScaleFactor to round up. If it rounds up, then the result of + // decimal.MaxValue * ScaleFactor - decimal.MinValue * ScaleFactor will still be + // larger than decimal.MaxValue. + const decimal OneHalfScaleFactor = 0.4999999999999999999999999999m; + + decimal halfRange = max * OneHalfScaleFactor - min * OneHalfScaleFactor; + + // Two reasons we're forced to use a scaled multiplier: + // + // 1. The range (max - min) is itself too large to store in decimal.MaxValue. + // 2. The result of result * (max - min) is too large to store in decimal.MaxValue. + // + // Check condition 1: + bool useScaledMultiplier = (halfRange > decimal.MaxValue * OneHalfScaleFactor); + + decimal multiplier = halfRange; + decimal divisor = 7.922816251426433759354395032m; // This value is the maximum value at maximum precision times (OneHalfScaleFactor / 0.5). + + // Check condition 2: + decimal inputMagnitude = Math.Abs(input); + + if (inputMagnitude >= 1.0m) + { + decimal maximumMultiplier = decimal.MaxValue / inputMagnitude; + + while (multiplier >= maximumMultiplier) + { + // Drop one digit of precision and try again. + multiplier *= 0.1m; + divisor *= 0.1m; + + useScaledMultiplier = true; + } + } + + // Step 2: Scale the value and adjust it to the desired range. This may decrease + // the accuracy by adjusting the scale as necessary, but we get the best possible + // outcome by starting with the most precise scale. + if (useScaledMultiplier) + { + decimal rangeMiddle = max * OneHalfScaleFactor + min * OneHalfScaleFactor; + + return rangeMiddle + input * multiplier / divisor; + } + else + { + // If we're dealing with values at the upper extreme ends (decimal.MinValue, decimal.MaxValue), + // then half their value can't be represented in a decimal and will be subject to rounding. The + // result of that rounding is that adding the values together will produce an out-of-range value. + // Therefore, we can't use a straightforward (min + max) * 0.5, or even (min * 0.5 + max * 0.5). + decimal rangeSize = max - min; + decimal rangeMiddle; + + if (min == max) + rangeMiddle = min; + else if (-min == max) + rangeMiddle = 0m; + else if (Math.Abs(min) < Math.Abs(max)) + rangeMiddle = max - rangeSize * 0.5m; + else + rangeMiddle = min + rangeSize * 0.5m; + + return rangeMiddle + input * rangeSize / 15.845632502852867518708790067m; + } + } + } +} \ No newline at end of file diff --git a/Source/Bogus/Randomizer.cs b/Source/Bogus/Randomizer.cs index 98f88a52..e27251d8 100644 --- a/Source/Bogus/Randomizer.cs +++ b/Source/Bogus/Randomizer.cs @@ -187,125 +187,6 @@ public double Double(double min = 0.0d, double max = 1.0d) } } - /// - /// Generates a decimal with maximum precision with uniform distribution across the range of - /// decimal values with maximum scaling: (decimal.MinValue .. decimal.MaxValue) divided by 10^28. - /// - /// - internal decimal GenerateRandomMaximumPrecisionDecimal() - { - // Decimal: 128 bits wide - // bit 0: sign bit - // bit 1-10: not used - // bit 11-15: scale (values 29, 30, 31 not used) - // bit 16-31: not used - // bit 32-127: mantissa (96 bits) - - // Max value: 00000000 FFFFFFFF FFFFFFFF FFFFFFFF - // = 79228162514264337593543950335 - - // Max value with max scaling: 001C0000 FFFFFFFF FFFFFFFF FFFFFFFF - // = 7.9228162514264337593543950335 - - int lowBits = Number(int.MinValue, int.MaxValue); - int middleBits = Number(int.MinValue, int.MaxValue); - int highBits = Number(int.MinValue, int.MaxValue); - - bool isNegative = Bool(); - - const int Scale = 28; - - return new decimal(lowBits, middleBits, highBits, isNegative, Scale); - } - - /// - /// Takes a decimal in the range in which decimal has maximum precision ((decimal.MinValue .. decimal.MaxValue) scaled by the maximum - /// scaling factor of 10^28) and transforms it into the caller's desired range, reducing precision only as much as needed. - /// - /// A decimal in the range (decimal.MinValue / SCALE .. decimal.MaxValue / SCALE), where SCALE is the maximum scaling factor (10^28). - /// The lower bound of the target range. - /// The upper bound of the target range. - /// - internal decimal ScaleMaximumPrecisionDecimalToRange(decimal input, decimal min, decimal max) - { - const decimal SmallestMaxPrecision = -7.9228162514264337593543950335m; - const decimal LargestMaxPrecision = 7.9228162514264337593543950335m; - - if (input <= SmallestMaxPrecision) - return min; - if (input >= LargestMaxPrecision) - return max; - - // Step 1: Figure out how much of the scale we can keep without causing an overflow. - // Note that the range can actually exceed decimal.MaxValue, e.g. if max is itself - // decimal.MaxValue and min is negative. So, we work with half the range, using this - // scale factor that is as close to 0.5 as possible without causing the result of - // decimal.MaxValue * ScaleFactor to round up. If it rounds up, then the result of - // decimal.MaxValue * ScaleFactor - decimal.MinValue * ScaleFactor will still be - // larger than decimal.MaxValue. - const decimal OneHalfScaleFactor = 0.4999999999999999999999999999m; - - decimal halfRange = max * OneHalfScaleFactor - min * OneHalfScaleFactor; - - // Two reasons we're forced to use a scaled multiplier: - // - // 1. The range (max - min) is itself too large to store in decimal.MaxValue. - // 2. The result of result * (max - min) is too large to store in decimal.MaxValue. - // - // Check condition 1: - bool useScaledMultiplier = (halfRange > decimal.MaxValue * OneHalfScaleFactor); - - decimal multiplier = halfRange; - decimal divisor = 7.922816251426433759354395032m; // This value is the maximum value at maximum precision times (OneHalfScaleFactor / 0.5). - - // Check condition 2: - decimal inputMagnitude = Math.Abs(input); - - if (inputMagnitude >= 1.0m) - { - decimal maximumMultiplier = decimal.MaxValue / inputMagnitude; - - while (multiplier >= maximumMultiplier) - { - // Drop one digit of precision and try again. - multiplier *= 0.1m; - divisor *= 0.1m; - - useScaledMultiplier = true; - } - } - - // Step 2: Scale the value and adjust it to the desired range. This may decrease - // the accuracy by adjusting the scale as necessary, but we get the best possible - // outcome by starting with the most precise scale. - if (useScaledMultiplier) - { - decimal rangeMiddle = max * OneHalfScaleFactor + min * OneHalfScaleFactor; - - return rangeMiddle + input * multiplier / divisor; - } - else - { - // If we're dealing with values at the upper extreme ends (decimal.MinValue, decimal.MaxValue), - // then half their value can't be represented in a decimal and will be subject to rounding. The - // result of that rounding is that adding the values together will produce an out-of-range value. - // Therefore, we can't use a straightforward (min + max) * 0.5, or even (min * 0.5 + max * 0.5). - decimal rangeSize = max - min; - decimal rangeMiddle; - - if (min == max) - rangeMiddle = min; - else if (-min == max) - rangeMiddle = 0m; - else if (Math.Abs(min) < Math.Abs(max)) - rangeMiddle = max - rangeSize * 0.5m; - else - rangeMiddle = min + rangeSize * 0.5m; - - return rangeMiddle + input * rangeSize / 15.845632502852867518708790067m; - } - } - /// /// Get a random decimal, between 0.0 and 1.0. /// @@ -313,17 +194,7 @@ internal decimal ScaleMaximumPrecisionDecimalToRange(decimal input, decimal min, /// Maximum, default 1.0 public decimal Decimal(decimal min = 0.0m, decimal max = 1.0m) { - if (min > max) - { - decimal tmp = min; - - min = max; - max = tmp; - } - - var unscaledSample = GenerateRandomMaximumPrecisionDecimal(); - - return ScaleMaximumPrecisionDecimalToRange(unscaledSample, min, max); + return Convert.ToDecimal(Double()) * (max - min) + min; } /// From 77af454ef769e393efad14d7616eccb6b0902312 Mon Sep 17 00:00:00 2001 From: bchavez Date: Sat, 15 Aug 2020 18:12:31 -0700 Subject: [PATCH 4/4] Redirect PR work to Decimal3 extension method. --- Source/Bogus.Tests/GitHubIssues/Issue319.cs | 53 +++++++ Source/Bogus.Tests/GitHubIssues/Issue319_2.cs | 48 ------ Source/Bogus.Tests/RandomizerTest.cs | 9 +- .../Extensions/ExtensionsForRandomizer.cs | 141 ++++++++++++++++- .../Extensions/ExtensionsForRandomizer2.cs | 142 ------------------ 5 files changed, 198 insertions(+), 195 deletions(-) delete mode 100644 Source/Bogus.Tests/GitHubIssues/Issue319_2.cs delete mode 100644 Source/Bogus/Extensions/ExtensionsForRandomizer2.cs diff --git a/Source/Bogus.Tests/GitHubIssues/Issue319.cs b/Source/Bogus.Tests/GitHubIssues/Issue319.cs index 19adf40c..c778808e 100644 --- a/Source/Bogus.Tests/GitHubIssues/Issue319.cs +++ b/Source/Bogus.Tests/GitHubIssues/Issue319.cs @@ -1,12 +1,22 @@ using System; +using System.Collections.Generic; +using System.Reflection; using Bogus.Extensions; using FluentAssertions; using Xunit; +using Xunit.Abstractions; +using Xunit.Sdk; namespace Bogus.Tests.GitHubIssues { public class Issue319 : SeededTest { + ITestOutputHelper _output; + + public Issue319(ITestOutputHelper output) + => _output = output; + + [Fact] public void can_generate_decimal_edge_case() { @@ -33,5 +43,48 @@ public void decimal2_should_throw_on_edge_case() a.Should().Throw(); } + + + class TestDataProvider : DataAttribute + { + public override IEnumerable GetData(MethodInfo testMethod) + { + yield return new object[] { 0m, decimal.MaxValue }; + yield return new object[] { decimal.MinValue, decimal.MaxValue }; + yield return new object[] { decimal.MinValue, 0m }; + // A range whose size exceeds decimal.MaxValue but which doesn't have decimal.MinValue or decimal.MaxValue as a bound. + yield return new object[] { decimal.MinValue * 0.6m, decimal.MaxValue * 0.6m }; + } + } + + [Theory, TestDataProvider] + public void decimal_with_very_large_range_succeeds(decimal min, decimal max) + { + var randomizer = new Randomizer(); + + for (int iteration = 0; iteration < 300; iteration++) + { + try + { + randomizer.Decimal3(min, max).Should().BeInRange(min, max); + } + catch + { + _output.WriteLine("Test failed on iteration {0}", iteration); + throw; + } + } + } + + + [Fact] + public void decimal3_fails_after_2_calls() + { + var r = new Randomizer(); + r.Decimal3(0m, decimal.MaxValue); + r.Decimal3(0m, decimal.MaxValue); + } + + } } \ No newline at end of file diff --git a/Source/Bogus.Tests/GitHubIssues/Issue319_2.cs b/Source/Bogus.Tests/GitHubIssues/Issue319_2.cs deleted file mode 100644 index e92ed5d8..00000000 --- a/Source/Bogus.Tests/GitHubIssues/Issue319_2.cs +++ /dev/null @@ -1,48 +0,0 @@ -using System.Collections.Generic; -using System.Reflection; -using FluentAssertions; -using Xunit; -using Xunit.Abstractions; -using Xunit.Sdk; - -namespace Bogus.Tests.GitHubIssues -{ - public class Issue319_2 : SeededTest - { - class TestDataProvider : DataAttribute - { - public override IEnumerable GetData(MethodInfo testMethod) - { - yield return new object[] { 0m, decimal.MaxValue }; - yield return new object[] { decimal.MinValue, decimal.MaxValue }; - yield return new object[] { decimal.MinValue, 0m }; - // A range whose size exceeds decimal.MaxValue but which doesn't have decimal.MinValue or decimal.MaxValue as a bound. - yield return new object[] { decimal.MinValue * 0.6m, decimal.MaxValue * 0.6m }; - } - } - - ITestOutputHelper _output; - - public Issue319_2(ITestOutputHelper output) - => _output = output; - - [Theory, TestDataProvider] - public void decimal_with_very_large_range_succeeds(decimal min, decimal max) - { - var randomizer = new Randomizer(); - - for (int iteration = 0; iteration < 300; iteration++) - { - try - { - randomizer.Decimal(min, max).Should().BeInRange(min, max); - } - catch - { - _output.WriteLine("Test failed on iteration {0}", iteration); - throw; - } - } - } - } -} diff --git a/Source/Bogus.Tests/RandomizerTest.cs b/Source/Bogus.Tests/RandomizerTest.cs index d1bd7539..7d96185c 100644 --- a/Source/Bogus.Tests/RandomizerTest.cs +++ b/Source/Bogus.Tests/RandomizerTest.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Reflection; using System.Text; +using Bogus.Extensions; using FluentAssertions; using Xunit; using Xunit.Abstractions; @@ -209,7 +210,7 @@ public void generate_maximum_range_decimal() { try { - r.GenerateRandomMaximumPrecisionDecimal().Should().BeInRange(SmallestMaxPrecision, LargestMaxPrecision); + ExtensionsForRandomizer.GenerateRandomMaximumPrecisionDecimal(r).Should().BeInRange(SmallestMaxPrecision, LargestMaxPrecision); } catch { @@ -240,9 +241,9 @@ public void scale_decimal_with_min_and_max(decimal min, decimal max, decimal mid const decimal SmallestMaxPrecision = -7.9228162514264337593543950335m; const decimal LargestMaxPrecision = 7.9228162514264337593543950335m; - r.ScaleMaximumPrecisionDecimalToRange(SmallestMaxPrecision, min, max).Should().Be(min); - r.ScaleMaximumPrecisionDecimalToRange(LargestMaxPrecision, min, max).Should().Be(max); - r.ScaleMaximumPrecisionDecimalToRange(0m, min, max).Should().Be(middle); + ExtensionsForRandomizer.ScaleMaximumPrecisionDecimalToRange(SmallestMaxPrecision, min, max).Should().Be(min); + ExtensionsForRandomizer.ScaleMaximumPrecisionDecimalToRange(LargestMaxPrecision, min, max).Should().Be(max); + ExtensionsForRandomizer.ScaleMaximumPrecisionDecimalToRange(0m, min, max).Should().Be(middle); } [Fact] diff --git a/Source/Bogus/Extensions/ExtensionsForRandomizer.cs b/Source/Bogus/Extensions/ExtensionsForRandomizer.cs index 97f82e37..1f73f05b 100644 --- a/Source/Bogus/Extensions/ExtensionsForRandomizer.cs +++ b/Source/Bogus/Extensions/ExtensionsForRandomizer.cs @@ -1,4 +1,6 @@ -namespace Bogus.Extensions +using System; + +namespace Bogus.Extensions { public static class ExtensionsForRandomizer { @@ -41,5 +43,142 @@ public static decimal Decimal2(this Randomizer r, decimal min = 0.0m, decimal ma // outcome by starting with the most precise scale. return result * (max - min) / 7.9228162514264337593543950335m + min; } + + + public static decimal Decimal3(this Randomizer r, decimal min = 0.0m, decimal max = 1.0m) + { + if (min > max) + { + decimal tmp = min; + + min = max; + max = tmp; + } + + var unscaledSample = GenerateRandomMaximumPrecisionDecimal(r); + + return ScaleMaximumPrecisionDecimalToRange(unscaledSample, min, max); + } + + + /// + /// Generates a decimal with maximum precision with uniform distribution across the range of + /// decimal values with maximum scaling: (decimal.MinValue .. decimal.MaxValue) divided by 10^28. + /// + /// + internal static decimal GenerateRandomMaximumPrecisionDecimal(Randomizer r) + { + // Decimal: 128 bits wide + // bit 0: sign bit + // bit 1-10: not used + // bit 11-15: scale (values 29, 30, 31 not used) + // bit 16-31: not used + // bit 32-127: mantissa (96 bits) + + // Max value: 00000000 FFFFFFFF FFFFFFFF FFFFFFFF + // = 79228162514264337593543950335 + + // Max value with max scaling: 001C0000 FFFFFFFF FFFFFFFF FFFFFFFF + // = 7.9228162514264337593543950335 + + int lowBits = r.Number(int.MinValue, int.MaxValue); + int middleBits = r.Number(int.MinValue, int.MaxValue); + int highBits = r.Number(int.MinValue, int.MaxValue); + + bool isNegative = r.Bool(); + + const int Scale = 28; + + return new decimal(lowBits, middleBits, highBits, isNegative, Scale); + } + + /// + /// Takes a decimal in the range in which decimal has maximum precision ((decimal.MinValue .. decimal.MaxValue) scaled by the maximum + /// scaling factor of 10^28) and transforms it into the caller's desired range, reducing precision only as much as needed. + /// + /// A decimal in the range (decimal.MinValue / SCALE .. decimal.MaxValue / SCALE), where SCALE is the maximum scaling factor (10^28). + /// The lower bound of the target range. + /// The upper bound of the target range. + /// + internal static decimal ScaleMaximumPrecisionDecimalToRange(decimal input, decimal min, decimal max) + { + const decimal SmallestMaxPrecision = -7.9228162514264337593543950335m; + const decimal LargestMaxPrecision = 7.9228162514264337593543950335m; + + if (input <= SmallestMaxPrecision) + return min; + if (input >= LargestMaxPrecision) + return max; + + // Step 1: Figure out how much of the scale we can keep without causing an overflow. + // Note that the range can actually exceed decimal.MaxValue, e.g. if max is itself + // decimal.MaxValue and min is negative. So, we work with half the range, using this + // scale factor that is as close to 0.5 as possible without causing the result of + // decimal.MaxValue * ScaleFactor to round up. If it rounds up, then the result of + // decimal.MaxValue * ScaleFactor - decimal.MinValue * ScaleFactor will still be + // larger than decimal.MaxValue. + const decimal OneHalfScaleFactor = 0.4999999999999999999999999999m; + + decimal halfRange = max * OneHalfScaleFactor - min * OneHalfScaleFactor; + + // Two reasons we're forced to use a scaled multiplier: + // + // 1. The range (max - min) is itself too large to store in decimal.MaxValue. + // 2. The result of result * (max - min) is too large to store in decimal.MaxValue. + // + // Check condition 1: + bool useScaledMultiplier = (halfRange > decimal.MaxValue * OneHalfScaleFactor); + + decimal multiplier = halfRange; + decimal divisor = 7.922816251426433759354395032m; // This value is the maximum value at maximum precision times (OneHalfScaleFactor / 0.5). + + // Check condition 2: + decimal inputMagnitude = Math.Abs(input); + + if (inputMagnitude >= 1.0m) + { + decimal maximumMultiplier = decimal.MaxValue / inputMagnitude; + + while (multiplier >= maximumMultiplier) + { + // Drop one digit of precision and try again. + multiplier *= 0.1m; + divisor *= 0.1m; + + useScaledMultiplier = true; + } + } + + // Step 2: Scale the value and adjust it to the desired range. This may decrease + // the accuracy by adjusting the scale as necessary, but we get the best possible + // outcome by starting with the most precise scale. + if (useScaledMultiplier) + { + decimal rangeMiddle = max * OneHalfScaleFactor + min * OneHalfScaleFactor; + + return rangeMiddle + input * multiplier / divisor; + } + else + { + // If we're dealing with values at the upper extreme ends (decimal.MinValue, decimal.MaxValue), + // then half their value can't be represented in a decimal and will be subject to rounding. The + // result of that rounding is that adding the values together will produce an out-of-range value. + // Therefore, we can't use a straightforward (min + max) * 0.5, or even (min * 0.5 + max * 0.5). + decimal rangeSize = max - min; + decimal rangeMiddle; + + if (min == max) + rangeMiddle = min; + else if (-min == max) + rangeMiddle = 0m; + else if (Math.Abs(min) < Math.Abs(max)) + rangeMiddle = max - rangeSize * 0.5m; + else + rangeMiddle = min + rangeSize * 0.5m; + + return rangeMiddle + input * rangeSize / 15.845632502852867518708790067m; + } + } + } } \ No newline at end of file diff --git a/Source/Bogus/Extensions/ExtensionsForRandomizer2.cs b/Source/Bogus/Extensions/ExtensionsForRandomizer2.cs deleted file mode 100644 index 007edeba..00000000 --- a/Source/Bogus/Extensions/ExtensionsForRandomizer2.cs +++ /dev/null @@ -1,142 +0,0 @@ -using System; - -namespace Bogus.Extensions -{ - public static class ExtensionsForRandomizer2 - { - public static decimal Decimal3(this Randomizer r, decimal min = 0.0m, decimal max = 1.0m) - { - if (min > max) - { - decimal tmp = min; - - min = max; - max = tmp; - } - - var unscaledSample = GenerateRandomMaximumPrecisionDecimal(r); - - return ScaleMaximumPrecisionDecimalToRange(unscaledSample, min, max); - } - - - /// - /// Generates a decimal with maximum precision with uniform distribution across the range of - /// decimal values with maximum scaling: (decimal.MinValue .. decimal.MaxValue) divided by 10^28. - /// - /// - internal static decimal GenerateRandomMaximumPrecisionDecimal(Randomizer r) - { - // Decimal: 128 bits wide - // bit 0: sign bit - // bit 1-10: not used - // bit 11-15: scale (values 29, 30, 31 not used) - // bit 16-31: not used - // bit 32-127: mantissa (96 bits) - - // Max value: 00000000 FFFFFFFF FFFFFFFF FFFFFFFF - // = 79228162514264337593543950335 - - // Max value with max scaling: 001C0000 FFFFFFFF FFFFFFFF FFFFFFFF - // = 7.9228162514264337593543950335 - - int lowBits = r.Number(int.MinValue, int.MaxValue); - int middleBits = r.Number(int.MinValue, int.MaxValue); - int highBits = r.Number(int.MinValue, int.MaxValue); - - bool isNegative = r.Bool(); - - const int Scale = 28; - - return new decimal(lowBits, middleBits, highBits, isNegative, Scale); - } - - /// - /// Takes a decimal in the range in which decimal has maximum precision ((decimal.MinValue .. decimal.MaxValue) scaled by the maximum - /// scaling factor of 10^28) and transforms it into the caller's desired range, reducing precision only as much as needed. - /// - /// A decimal in the range (decimal.MinValue / SCALE .. decimal.MaxValue / SCALE), where SCALE is the maximum scaling factor (10^28). - /// The lower bound of the target range. - /// The upper bound of the target range. - /// - internal static decimal ScaleMaximumPrecisionDecimalToRange(decimal input, decimal min, decimal max) - { - const decimal SmallestMaxPrecision = -7.9228162514264337593543950335m; - const decimal LargestMaxPrecision = 7.9228162514264337593543950335m; - - if (input <= SmallestMaxPrecision) - return min; - if (input >= LargestMaxPrecision) - return max; - - // Step 1: Figure out how much of the scale we can keep without causing an overflow. - // Note that the range can actually exceed decimal.MaxValue, e.g. if max is itself - // decimal.MaxValue and min is negative. So, we work with half the range, using this - // scale factor that is as close to 0.5 as possible without causing the result of - // decimal.MaxValue * ScaleFactor to round up. If it rounds up, then the result of - // decimal.MaxValue * ScaleFactor - decimal.MinValue * ScaleFactor will still be - // larger than decimal.MaxValue. - const decimal OneHalfScaleFactor = 0.4999999999999999999999999999m; - - decimal halfRange = max * OneHalfScaleFactor - min * OneHalfScaleFactor; - - // Two reasons we're forced to use a scaled multiplier: - // - // 1. The range (max - min) is itself too large to store in decimal.MaxValue. - // 2. The result of result * (max - min) is too large to store in decimal.MaxValue. - // - // Check condition 1: - bool useScaledMultiplier = (halfRange > decimal.MaxValue * OneHalfScaleFactor); - - decimal multiplier = halfRange; - decimal divisor = 7.922816251426433759354395032m; // This value is the maximum value at maximum precision times (OneHalfScaleFactor / 0.5). - - // Check condition 2: - decimal inputMagnitude = Math.Abs(input); - - if (inputMagnitude >= 1.0m) - { - decimal maximumMultiplier = decimal.MaxValue / inputMagnitude; - - while (multiplier >= maximumMultiplier) - { - // Drop one digit of precision and try again. - multiplier *= 0.1m; - divisor *= 0.1m; - - useScaledMultiplier = true; - } - } - - // Step 2: Scale the value and adjust it to the desired range. This may decrease - // the accuracy by adjusting the scale as necessary, but we get the best possible - // outcome by starting with the most precise scale. - if (useScaledMultiplier) - { - decimal rangeMiddle = max * OneHalfScaleFactor + min * OneHalfScaleFactor; - - return rangeMiddle + input * multiplier / divisor; - } - else - { - // If we're dealing with values at the upper extreme ends (decimal.MinValue, decimal.MaxValue), - // then half their value can't be represented in a decimal and will be subject to rounding. The - // result of that rounding is that adding the values together will produce an out-of-range value. - // Therefore, we can't use a straightforward (min + max) * 0.5, or even (min * 0.5 + max * 0.5). - decimal rangeSize = max - min; - decimal rangeMiddle; - - if (min == max) - rangeMiddle = min; - else if (-min == max) - rangeMiddle = 0m; - else if (Math.Abs(min) < Math.Abs(max)) - rangeMiddle = max - rangeSize * 0.5m; - else - rangeMiddle = min + rangeSize * 0.5m; - - return rangeMiddle + input * rangeSize / 15.845632502852867518708790067m; - } - } - } -} \ No newline at end of file