Skip to content

Require .upperBound - .lowerBound be finite for FloatingPoint random #17833

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 9, 2018
Merged

Require .upperBound - .lowerBound be finite for FloatingPoint random #17833

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c5ee1a3
Require that .upperBound - .lowerBound be finite for FloatingPoint ra…
stephentyrone Jul 9, 2018
b8da31f
Minor perf improvements while we're here.
stephentyrone Jul 9, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions stdlib/public/core/FloatingPoint.swift.gyb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2411,7 +2411,7 @@ where Self.RawSignificand : FixedWidthInteger {
/// - Parameters:
/// - range: The range in which to create a random value.
% if Range == 'Range':
/// `range` must not be empty.
/// `range` must not be empty and finite.
% end
/// - generator: The random number generator to use when creating the
/// new random value.
Expand All @@ -2426,6 +2426,15 @@ where Self.RawSignificand : FixedWidthInteger {
"Can't get random value with an empty range"
)
let delta = range.upperBound - range.lowerBound
// TODO: this still isn't quite right, because the computation of delta
// can overflow (e.g. if .upperBound = .maximumFiniteMagnitude and
// .lowerBound = -.upperBound); this should be re-written with an
// algorithm that handles that case correctly, but this precondition
// is an acceptable short-term fix.
_precondition(
delta.isFinite,
"There is no uniform distribution on an infinite range"
)
let rand: Self.RawSignificand
if Self.RawSignificand.bitWidth == Self.significandBitCount + 1 {
rand = generator.next()
Expand All @@ -2439,15 +2448,18 @@ where Self.RawSignificand : FixedWidthInteger {
let significandCount = Self.significandBitCount + 1
let maxSignificand: Self.RawSignificand = 1 << significandCount
% if 'Closed' not in Range:
rand = generator.next(upperBound: maxSignificand)
// Rather than use .next(upperBound:), which has to work with arbitrary
// upper bounds, and therefore does extra work to avoid bias, we can take
// a shortcut because we know that maxSignificand is a power of two.
rand = generator.next() & (maxSignificand - 1)
% else:
rand = generator.next(upperBound: maxSignificand + 1)
if rand == maxSignificand {
return range.upperBound
}
% end
}
let unitRandom = Self.init(rand) * Self.ulpOfOne / 2
let unitRandom = Self.init(rand) * (Self.ulpOfOne / 2)
let randFloat = delta * unitRandom + range.lowerBound
% if 'Closed' not in Range:
if randFloat == range.upperBound {
Expand Down Expand Up @@ -2482,7 +2494,7 @@ where Self.RawSignificand : FixedWidthInteger {
///
/// - Parameter range: The range in which to create a random value.
% if Range == 'Range':
/// `range` must not be empty.
/// `range` must not be empty and finite.
% end
/// - Returns: A random value within the bounds of `range`.
@inlinable
Expand Down