nick8325 / quickcheck

Automatic testing of Haskell programs.
Other
713 stars 119 forks source link

WIP: Add random flag to drop 'random' dependency #292

Closed phadej closed 4 years ago

phadej commented 4 years ago

There is a random refactoring work going on, and it's API will change drastically. See https://github.com/idontgetoutmuch/random/ cc @lehins

This PR is a work in progress to see how much one can do just with splitmix: Turns out quite a lot. We "only" lose

The question is that when new random will comes out, what configuration options QuickCheck should support (and how) out of:

It looks like that the default random generator of new random will be splitmix. Therefore, we could have two flags:

flag splitmix
  description: Use splitmix as random number generator

flag random
  description: Add `random` utilities. This flag should be set to `True`, if `splitmix` is False.

Making three configurations:

splitmix = True,  random = True    -- default setting
splitmix = True,  random = False
splitmix = False, random = True

As all this is complicated, this is why I open this discussion now. I think we could even make this change before new random version is released, then adopting to new random (and splitmix which would drop random dependency due https://github.com/phadej/splitmix/issues/34) more straightforward.

Adding the new flags won't even be a breaking change, as with their default setting won't change the public API.


One to do item is to write nextInteger for splitmix (which would select uniformly from the range of two Integers). I'll do that soon.

Some other small changes:

phadej commented 4 years ago

One option is to make splitmix compile with Hugs. I don't think that's hard. I'll try that (but looks like I have problems compiling Hugs-sep2006 on my machine, so it's a bit tricky).

phadej commented 4 years ago

And it seems I can make splitmix hugs friendly, if I figure out how to do overflowing arithmetic on Word64s:

System.Random.SplitMix> let x = 0xffffffff :: Word64 in x * x
18446744065119617025

System.Random.SplitMix> let x = 0xffffffff :: Word64 in x * x * x

Program error: arithmetic overflow
lehins commented 4 years ago

@phadej Despite that the API of new version of random is drastically improved, it is still mostly backwards compatible, so you probably don't need to make to many changes to QuickCheck.

One thing that definitely is not backwards compatible, is that we dropped support for Hugs compiler, because it is just dead in a water and we don't want it to drag us behind.

If you are curious at what changes had to be done to all available pure RNGs in Haskell in order to support new random, then check out the list of PRs in this : https://github.com/idontgetoutmuch/random/issues/23#issuecomment-613755492

Edit Note that these changes listed in the comment are not required in order for those libraries to compile and work with new random, they are desired to get full performance benefits

phadej commented 4 years ago

@lehins what are your plans to for GHCs support, I see you have base >=4.10 bound, which means GHC 8.2+. I think QuickCheck could rely on older random for older GHCs, if the API is indeed the same for the bits QuickCheck uses (which is essentially Random class).

lehins commented 4 years ago

Yeah I think that would be a good approach:

lehins commented 4 years ago

@phadej You asked at some point: https://github.com/idontgetoutmuch/random/pull/1#discussion_r392311303

Don't these classes[Uniform+UniformRange] essentially deprecate whole of Random, or what purpose Random would serve?

The answer is they could deprecate it, but we want to can keep Random around for backwards compatibility. Also Random has instances that simply wouldn't make sense for Uniform for example, such as Integer or Double

Anyways, the benefit of this decision is, not much needs to be done to switch to new random, both for users and RNG maintainers