Infinities as floating point range endpoints

[curiousleo]

Fixes #54. Brings the behaviour in line with the docs, which state that if exactly one endpoint is infinite, then uniformRM returns that endpoint.

[curiousleo]

This was previously the case too, tested here:

random/src/System/Random/Stateful.hs

Lines 608 to 611 in 11464aa

	-- * If \(a\) is @-Infinity@ and \(b\) is @Infinity@, the result is @NaN@.
	--
	-- >>> let (a, b, x) = (-inf, inf, 0.5) in x * a + (1 - x) * b
	-- NaN

But we need this guard now so we can distinguish "both endpoints are unequal infinities" (this case) from "one of the endpoints is an infinity" (the next two cases)..

[curiousleo]

This test fails without the additional guards in uniformRM, and passes when they are added.

[curiousleo]

The benchmarks show an unacceptable decline in performance. I'll turn this back into a draft until I've figured this out.

Before (11464aa):

pure/random/Float                        mean 27.71 μs  ( +- 555.9 ns  )
pure/uniformR/unbounded/Float            mean 31.27 μs  ( +- 3.095 μs  )

Currently (ef2ee10):

pure/random/Float                        mean 15.57 ms  ( +- 2.282 ms  )
pure/uniformR/unbounded/Float            mean 15.89 ms  ( +- 2.239 ms  )

Edit: see #68 (comment) for updated benchmarks.

[curiousleo]

64c52a5 makes Float perform as before, but Double takes twice as long as the baseline ... (Edit: I've optimised the code so this is no longer the case.)

Before (master / 11464aa):

pure/random/Float                        mean 31.08 μs  ( +- 4.587 μs  )
pure/uniformR/unbounded/Float            mean 31.72 μs  ( +- 3.938 μs  )
pure/uniformR/floating/pure/uniformFloat01M mean 30.22 μs  ( +- 3.292 μs  )
pure/uniformR/floating/pure/uniformFloatPositive01M mean 28.35 μs  ( +- 1.074 μs  )

pure/random/Double                       mean 28.02 μs  ( +- 540.9 ns  )
pure/uniformR/unbounded/Double           mean 31.69 μs  ( +- 2.741 μs  )
pure/uniformR/floating/pure/uniformDouble01M mean 30.83 μs  ( +- 2.935 μs  )
pure/uniformR/floating/pure/uniformDoublePositive01M mean 28.66 μs  ( +- 1.471 μs  )

After 67e12bf:

pure/random/Float                        mean 28.02 μs  ( +- 853.7 ns  )
pure/uniformR/unbounded/Float            mean 31.87 μs  ( +- 3.505 μs  )
pure/uniformR/floating/pure/uniformFloat01M mean 27.99 μs  ( +- 1.198 μs  )
pure/uniformR/floating/pure/uniformFloatPositive01M mean 29.99 μs  ( +- 3.298 μs  )

pure/random/Double                       mean 28.22 μs  ( +- 795.3 ns  )
pure/uniformR/unbounded/Double           mean 31.90 μs  ( +- 4.201 μs  )
pure/uniformR/floating/pure/uniformDouble01M mean 31.09 μs  ( +- 3.051 μs  )
pure/uniformR/floating/pure/uniformDoublePositive01M mean 31.19 μs  ( +- 3.234 μs  )

[curiousleo]

Performance issues fixed.

[Shimuuar]

makes Float perform as before, but Double takes twice as long as the baseline ...

I've observed such behavior when I implemented uniformDouble01M changes such as adding/removing benchmark led to 2x performance jumps. It looks like GHC might run or skip some optimization

[curiousleo]

Ready for review.

[idontgetoutmuch]

I would write positiveInf = 1 / 0 then you don't need the Read constraint but I don't know if this is good practice.

[Shimuuar]

It is certainly much faster. Probably even constant-folded

[lehins]

Considering that it is in a test suite, I don't think performance matters much. Using read makes it a bit more obvious I think.

[Bodigrim]

I think this is ready to be merged. @Shimuuar @idontgetoutmuch if either of you has a strong opinion about changing read "Infinity", please voice it.

[Shimuuar]

Sorry, I forgot about this PR. Why NaN is returned when both endpoints are infinities? Following definition (in pseudocode) seems more sensible:

uniformRM (+∞,+∞) _ = return +∞
uniformRM (-∞,-∞) _ = return -∞
uniformRM (-∞,+∞) _ = oneOf [-∞,+∞]

Also number of ifs on happy path (neither endpoint is ∞) worry me a bit.

[Bodigrim]

Why NaN is returned when both endpoints are infinities?

This has been discussed back in #54. I agree with @idontgetoutmuch that returning NaN is more reasonable than alternation between infinities.

[Bodigrim]

From my point of view the issue in #54 is not that much about returning NaNs per se, but about returning NaNs inconsistently, only in a tiny fraction of cases, so small, that it would easily pass a test suite of 100 random values. It is worth to change this behaviour a) to be consistent with the majority of cases, b) to match our own documentation: http://hackage.haskell.org/package/random-1.2.0/docs/System-Random-Stateful.html#g:14

On the other hand, uniformRM (-∞,+∞) returns NaN consistently and is clearly documented ibidem. I do not see much reason to change this behaviour.

[Shimuuar]

I just found this behavior mildly surprising which is fine for floating point. What isn't when it comes to floating point.

BTW I came up with optimization which exploits quirks of floating point arithmetics:

  | isInfinite l || isInfinite h = return $! h + l

If only one of l,h is infinite they'll absorb finite counterpart and (-∞) + (+∞) = NaN. It bring implementation from 4 ifs to only 2 ifs.

Otherwise LGTM

[Bodigrim]

@curiousleo what do you think about @Shimuuar's suggestion? Also, could you please rebase?

[curiousleo]

@curiousleo what do you think about @Shimuuar's suggestion? Also, could you please rebase?

Thanks for the reminder. The suggestion looks neat! I'll come back to this PR as soon as free time permits.