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

Please provide instance for (Enum a, Bounded a) for Random #21

Closed
kvanbere opened this issue Oct 4, 2014 · 15 comments · Fixed by #104
Closed

Please provide instance for (Enum a, Bounded a) for Random #21

kvanbere opened this issue Oct 4, 2014 · 15 comments · Fixed by #104

Comments

@kvanbere
Copy link

kvanbere commented Oct 4, 2014

It would be really nice to be able to do something like this:

> data Action = Code | Eat | Sleep deriving (Bounded, Enum)
> randomR :: IO Action
Eat

The implementation for (Enum a, Bounded a) => can cheat a little to get a valid range (psuedo, for randomRIO since it's easier/shorter to show):

let lower = fromEnum (minBound :: a)
let upper = fromEnum (maxBound :: a)
return $ toEnum <$> randomRIO (lower, upper)

Note the usage of the underlying instance for Int to select a constructor.

It would have been magical if we could have just a Enum a => instance, but I can't think of a way to do it.

@kvanbere kvanbere changed the title Please provide instance for (Enum a, Bounded a) Please provide instance for (Enum a, Bounded a) for Random Oct 4, 2014
@rwbarton
Copy link

rwbarton commented Oct 4, 2014

What instance do you have in mind, instance (Bounded a, Enum a) => Random a where ...? There can't be such an instance, since it would overlap with every other instance.

@kvanbere
Copy link
Author

kvanbere commented Oct 4, 2014

I thought there was a most specific instance rule?

The instance would just pick a random constructor from those available (range known by Bounded), evenly distributed among all the enumerations (defined by Enum). The implementation is not hard, I just haven't written it or opened a PR or anything yet.

Essentially, the best case scenario would be if the compiler could derive Random, but it can't, so having an Enum/Bounded instance is the closest thing to it, since it will derive those automatically for you.

@cartazio
Copy link
Contributor

cartazio commented Oct 4, 2014

This would prevent any user derived instances. If you manufactured a newtype for just giving this instance, that might be more tenable. But even then, this also implicitly assumes a uniform distribution is always the correct default, and I'm not sure if thats true! (Or that every Bounded instance is a properly lawful one, which I'd hope but wouldnt want to impose.)

The analogous monoids induced by Num use Sum and Product newtypes, so i think any analogous choice here would needs be using the same strategy. like UniformBounded newtype or some such

@kvanbere
Copy link
Author

kvanbere commented Oct 5, 2014

I kind of got the impression that the Random typeclass implied uniform, but no?

At least, all the existing instances seem to be uniform, and I can't think of a case where you wouldn't just plug something on top of the linear distr to give the different distribution you need.

@emmanueltouzery
Copy link

A reasonable default instance is:

instance Random Action where
  randomR (a, b) g =
    case randomR (fromEnum a, fromEnum b) g of
      (x, g') -> (toEnum x, g')
  random g = randomR (minBound, maxBound) g

It's tricky boilerplate that would be easy to get wrong and that you just copy-paste.

Maybe Random could export:

defaultBoundedEnumRandomR :: (Bounded a, Enum a, RandomGen g) => (a, a) -> g -> (a, g)
defaultBoundedEnumRandomR (a, b) g =
  case randomR (fromEnum a, fromEnum b) g of
    (x, g') -> (toEnum x, g')

defaultBoundedEnumRandom :: (Bounded a, Enum a, RandomGen g) => g -> (a, g)
defaultBoundedEnumRandom g = defaultBoundedEnumRandomR (minBound, maxBound) g

And then users could simply do:

instance Random Action where
    randomR = defaultBoundedEnumRandomR
    random = defaultBoundedEnumRandom

@cartazio
Copy link
Contributor

exporting some default methods might be good. interesting idea!

@rwbarton
Copy link

defaultBoundedEnumRandomR :: (Bounded a, Enum a, RandomGen g) => (a, a) -> g -> (a, g)
defaultBoundedEnumRandom :: (Bounded a, Enum a, RandomGen g) => g -> (a, g)

These would also be useful to use directly (rather than in a Random instance): suppose you import a type that is an instance of Bounded and Enum but not Random, and you don't want to define an orphan Random instance. So, we could just leave default out of the name.

The first one doesn't require Bounded either, so we could drop that from the name too.

@buggymcbugfix
Copy link

This would be really nice to have.

@kvanbere
Copy link
Author

Ah yes, another +1 on this .. it’s 3 year’s old, I had forgotten all about it .

@kvanbere
Copy link
Author

Is PR accepted ?

@cartazio
Copy link
Contributor

cartazio commented Oct 31, 2017 via email

@Bodigrim
Copy link
Contributor

I'm not convinced that sprinkling a couple of fromEnum / toEnum is that much of boilerplate. It certainly does not justify introducing overlapping instances.

However, I would not particularly oppose a PR, introducing uniformBoundedEnum :: (Enum a, Bounded a, StatefulGen g m) => g -> m a or something similar.

@Shimuuar
Copy link
Contributor

I think we should introduce newtype wrapper which allows to derive Uniform/UniformRange instances using DerivingVia

newtype UniformEnum a = UniformEnum a

instance Enum a => UniformRange (UnniformEnum a) where
  ...
instance (Bounded a, Enum a) => Uniform (UnniformEnum a) where
  ...

data RGB = R | G | B
  deriving (Enum.Bounded)
  deriving Uniform via UniformEnum RGB

@chessai
Copy link
Member

chessai commented Jun 26, 2020

@Shimuuar that looks good to me!

@Bodigrim
Copy link
Contributor

Bodigrim commented May 8, 2021

Closing as done in #104. Feel free to reopen if there is a specific motivation to use this feature via old Random interface.

@Bodigrim Bodigrim closed this as completed May 8, 2021
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 a pull request may close this issue.

8 participants