Postpone: Add MonadThrow to UniformRange

[lehins]

For now uniformRM doesn't fail any where, this PR just prepares the types for failure

[curiousleo]

Thanks for this proposal!

This would break two examples in the top level docs (which I haven't been able to hook up to doctests), please fix those:

random/random.cabal

Line 25 in bb2d26a

>>> take 10 (rolls pureGen) :: [Word8]

random/random.cabal

Lines 40 to 42 in bb2d26a

	>>> let rollM = uniformRM (1, 6) :: MonadRandom g s m => g s -> m Word8
	>>> let pureGen = mkStdGen 42
	>>> runGenState_ pureGen (replicateM 10 . rollM) :: m [Word8]

I would also like to see confirmation that this does not impact performance in a big way. I just merged #114 which should make it easier to compare a benchmark run off interface-to-performance with a benchmark run off this branch.

[curiousleo]

This PR suggests throwing a synchronous exception when the caller violates a precondition in the interval definition, e.g. the precondition "do not use generator functions with an empty interval".

Arnaud Spiwack's recent blog post contains this in the list of circumstances where one should use imprecise exceptions (error):

I'm writing a function which has a precondition that the typechecker can't express

(https://www.tweag.io/posts/2020-04-16-exceptions-in-haskell.html)

This suggests that we should use an imprecise exception (error) here instead, since we're dealing with unmet preconditions.

Attempting to generate values in an empty interval does sound like a programming error to me, which suggests that user code should validate that it is using sensible bounds before it calls uniformR.

What do you think? E.g. what good code patterns do synchronous exceptions enable?

[lehins]

@curiousleo I'll start with the fact that I don't agree with Arnaud Spiwack on usage of imprecise exceptions. Taking division by zero as an example. In order to throw error on y=0 in div x y there is an explicit check if y==0 in base, but in order for the user to avoid call to error he needs to check y==0 once more. Therefore we have a redundant check, but if result of div was Maybe that check could have been avoided. And because ghc is really good at optimizing Maybe away it would actually work faster. Moreover we have partial functions all over the place and smart people try to avoid them, so your suggestion of using error in pure code is something I would never agree with.

The only time I accept usage of error is in test suites and when there is no other way. Example of latter would be error in split, without restructuring and breaking bunch of code we cannot avoid error for non-splittable.

I would be ok with switching to Maybe for pure code, if MonadThrow seems too much:

uniformR :: (RandomGen g, UniformRange a) => (a, a) -> g -> Maybe (a, g)

Another thing I never got a chance to comment on is I think it was a bad idea to place examples into cabal file. Description of package should be high level instead of a short tutorial. Moreover these examples are available in haddock, so I really see no point in duplicating them

[lehins]

One more thing I forgot to mention. If I am writing a reusable function without knowing the actual type of the data that will be generated, I do not know ahead of time what input I should check to uniformR in order to prevent it from failing. If I were to use error I have no way of protecting my code from runtime exception, except by using uniformR in IO and catching UserError which is really terrible.

[Shimuuar]

I want to point out that depending on m instantiation MonadThrow's throwM could either raise synchronous exception: anything on top of IO, or imprecise exceptions

It's also trivially easy to convert MonadThrow based code to code that just throws exceptions around. Just use

newtype Partial a = Partial { partial :: a }
  deriving Functor,Applicative,Monad via Identity a

instance MonadThrow Partial where
  throwM = throw

EDIT: Maybe/Either, exceptions. What did I think?

[curiousleo]

In light of #113 (comment) and #113 (comment), I think we can shelve this for now: AFAICT this PR prepares the way to a clusivity API, which we have decided not to focus on for v1.2.

The discussion that's already happened here is still valuable once we pick up clusivity API work again, so I'm happy to leave this open - as you prefer. If so, perhaps mark it as [Inactive] or something like that in the PR title to reflect this status.