Skip to content
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.

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

WIP: Implement safetey around non-splittable PRNGs #94

Closed
wants to merge 1 commit into from

Conversation

lehins
Copy link
Contributor

@lehins lehins commented Jan 27, 2021

I think I figured out an elegant solution for differentiating splittable vs non-splittable PRNGs in a type safe way with very little breakage!!!

CC @idontgetoutmuch and @curiousleo We've discussed the problem here at some lengths: idontgetoutmuch#7

This is the change to the type class:

class RandomGen g where
  type Splittable g :: Bool
  type Splittable g = 'True
  ...
  split :: Splittable g ~ 'True => g -> (g, g)

Which means only instances for PRNGs that are non splittable need to be adjusted, eg:

instance RandomGen PureMT where
   type Splittable PureMT = TypeError ('Text "PureMT is not a splittable PRNG")
   split = error "System.Random.Mersenne.Pure: unable to split the mersenne twister"

which if you try to use split function, will give you a nice error message:

impossibleSplit :: IO (PureMT, PureMT)
impossibleSplit = split <$> newPureMT
    • PureMT is not a splittable PRNG
    • In the first argument of ‘(<$>)’, namely ‘split’
      In the expression: split <$> newPureMT
      In an equation for ‘impossibleSplit’:
          impossibleSplit = split <$> newPureMT
   |
Compilation failed.

Instead of this ugly runtime error:

*** Exception: System.Random.Mersenne.Pure: unable to split the mersenne twister
CallStack (from HasCallStack):

@curiousleo
Copy link
Contributor

That looks very nice!

It does seem to break deriving on old LTS. But that looks fixable. Other than that, I don't see any downsides with this approach.

It may be worth circulating this on the libraries list: (1) this technique deserves to be more well-known, (2) perhaps the authors of PRNG implementations have feedback.

@Shimuuar
Copy link
Contributor

That's cute hack!

I don't think it's possible without dropping either type family or GHC<8.2. Prior to 8.2 GHC could derive newtype instances for classes with associated types. See https://gitlab.haskell.org/ghc/ghc/-/issues/2721

@lehins
Copy link
Contributor Author

lehins commented Jan 28, 2021

We can manually provide those instances, they don't have to be derived.

@idontgetoutmuch
Copy link
Member

Nice :)

@lehins
Copy link
Contributor Author

lehins commented Sep 19, 2021

I figured how to improve it even further.

class RandomGen g where
  type Splittable g :: Constraint
  type Splittable g =
    TypeError ('ShowType g ':<>: 'Text " is not a splittable RandomGen")
  ...
  split :: Splittable g => g -> (g, g)

Any generator instance that is actually splittable only has to override the TypeError with ():

instance RandomGen SM.SMGen where
  type Splittable SM.SMGen = ()
  ...
  split = SM.splitSMGen

Here what we get in ghci:

> data NoopGen = NoopGen
> :{
| instance RandomGen NoopGen where
|   genWord64 _ = (0, NoopGen)
|   split _ = error "Impossible"
| :}
> split NoopGen

<interactive>:8:1: error:
     NoopGen is not a splittable RandomGen
     In the expression: split NoopGen
      In an equation for it’: it = split NoopGen

CC @treeowl You've expressed interest in this in #97, what do you think about this approach?

Naturally, this is a backwards incompatible solution, but was always meant to be a breaking change. It can be included in random-1.3.0 whenever that might happen.

@lehins
Copy link
Contributor Author

lehins commented Jan 27, 2024

Closing in favor of #160

@lehins lehins closed this Jan 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants