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

Support non-constant arbitrary functions #171

Merged
merged 12 commits into from
Jun 21, 2015

Conversation

non
Copy link
Contributor

@non non commented Jun 12, 2015

This PR adds support for better Arbitrary[FunctionN[...]] instances. It does a few things:

  1. Removes most use of scala.util.Random.
  2. Implement a simple linear-congruent RNG (Knuth's MMIX), that uses 64-bits of state.
  3. Change Gen methods to explicitly receive (and pass) RNG seeds (Long values)
  4. Introduce Cogen type to permutate RNG seed states.
  5. Introduce a bunch of implicit Cogen instances for many built-in types.
  6. Use codegen to wire up new Gen and Arbitrary instances for functions
  7. Add a line to build.sbt to get tests running faster for ScalaJS.

There is a bit of sketchiness in Gen due to its use of Option[A] instead of A. I was forced to use .get (like a few other existing methods do) which is not ideal. I can't tell if you'd expect this to be a major problem for ScalaCheck users.

We should also add a bunch more Cogen instances. I chose not to do this until I got a sense of whether this PR was going in the right direction.

Finally, while I bet most people using arbitrary functions would use primitive types, there is a bunch of work around updating documentation to explain this approach (and how to define custom Cogen instances).

Fixes #136

@non
Copy link
Contributor Author

non commented Jun 12, 2015

Argh! Tests passed locally -- will investigate why they are failing for Travis.

@non
Copy link
Contributor Author

non commented Jun 12, 2015

Oh, OK. These are all MIMA failures.

This change is definitely not binary-compatible.

@xuwei-k
Copy link
Contributor

xuwei-k commented Jun 12, 2015

awesome! But maybe there is a same issue as my library.

scalaprops/scalaprops#6

scala> Gen.listOfN(10, implicitly[Arbitrary[Int => Int]].arbitrary).sample.get.map((1 to 10).map(_)) foreach println
Vector(-2147483648, 1, -2147483648, 205452885, 2043426884, 1, -1632521114, 1, -2147483648, 243620675)
Vector(-2147483648, 1, -2147483648, 205452885, 2043426884, 1, -1632521114, 1, -2147483648, 243620675)
Vector(-2147483648, 1, -2147483648, 205452885, 2043426884, 1, -1632521114, 1, -2147483648, 243620675)
Vector(-2147483648, 1, -2147483648, 205452885, 2043426884, 1, -1632521114, 1, -2147483648, 243620675)
Vector(-2147483648, 1, -2147483648, 205452885, 2043426884, 1, -1632521114, 1, -2147483648, 243620675)
Vector(-2147483648, 1, -2147483648, 205452885, 2043426884, 1, -1632521114, 1, -2147483648, 243620675)
Vector(-2147483648, 1, -2147483648, 205452885, 2043426884, 1, -1632521114, 1, -2147483648, 243620675)
Vector(-2147483648, 1, -2147483648, 205452885, 2043426884, 1, -1632521114, 1, -2147483648, 243620675)
Vector(-2147483648, 1, -2147483648, 205452885, 2043426884, 1, -1632521114, 1, -2147483648, 243620675)
Vector(-2147483648, 1, -2147483648, 205452885, 2043426884, 1, -1632521114, 1, -2147483648, 243620675)

@non
Copy link
Contributor Author

non commented Jun 13, 2015

Ouch! Good catch. Yes, that's definitely a bug.

import language.higherKinds
import language.implicitConversions

sealed trait Cogen[-T] {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sealed abstract class is better for keep binary compatibility.

@xuwei-k
Copy link
Contributor

xuwei-k commented Jun 13, 2015

following expression should returns List((false,false), (false,true), (true,false), (true,true))

scala> org.scalacheck.Gen.listOfN(100000, implicitly[Arbitrary[Boolean => Boolean]].arbitrary).sample.get.map{f => (f(true), f(false))}.distinct.sorted
res0: List[(Boolean, Boolean)] = List((false,true), (true,false))

scala> org.scalacheck.Gen.listOfN(100000, implicitly[Arbitrary[Boolean => Boolean]].arbitrary).sample.get.map{f => (f(true), f(false))}.distinct.sorted
res1: List[(Boolean, Boolean)] = List((false,true), (true,false))

scala> org.scalacheck.Gen.listOfN(100000, implicitly[Arbitrary[Boolean => Boolean]].arbitrary).sample.get.map{f => (f(true), f(false))}.distinct.sorted
res2: List[(Boolean, Boolean)] = List((false,true), (true,false))

@rickynils
Copy link
Contributor

@non This is great stuff! I have worked in a similar direction in the scalacheck2 branch, but there I've made even more changes to Gen to also support finite generators. Due to lack of time, lots of stuff remains to be done there, though.

So the question is how to go forward. The realistic approach is probably to try to get the immutable gen and cogen into a 1.13 release, and keep my other changes for 2.0.

I'll make an attempt to dig into your solution as soon as possible.

@non
Copy link
Contributor Author

non commented Jun 13, 2015

There does seem to be a problem with Gen.oneOf combined with functions, which is demonstrated by @xuwei-k's second issue. I'm looking into it.

@rickynils that sounds fine. Let me know if you see any issues or want me to make any changes.

@rickynils
Copy link
Contributor

@non If you rebase this PR, you can bump versionNumber in build.sbt to 1.13.0, and set previousArtifact to None. That should take care of the failing Travis tests.

non added 9 commits June 16, 2015 13:52
This commit introduces a (seed: Long) parameter to many methods
that need to generate random values, as well as an Rng object
that implements a simple linear-congruent RNG.

The jvm tests pass. Currently getting permgen/OOM errors in the
js tests, not sure if this indicates a problem with the
implementation.

Still to do: there are still probably places where seeds are
not being threaded through correctly.
This type allows values to perturb the RNG, meaning that we can
build something resembling a true Gen[A => B]. The basic strategy
is as follows:

Given a Cogen[A] and a Gen[B], we can create an A => B. When we
are given a particular A value, we use Cogen[A] to permute the
RNG seed we have, and then use Gen[B] to generate a B value.
In this way, the B values created are dependent on the A values
provided, and we have a "real" function.

This commit doesn't actually do any of that yet, but it does
introduce a Cogen[A] type, and define a bunch of useful instances.
There is some ugliness in here (for example, .retrieve.get is a
pattern I've copied from elsewhere in the code). However, the
instances I've produced do seem to work.

The next step is to figure out how to use code-gen to create these
methods, as well as updating the Arbitrary code.
This commit adds support for code-generation for Gen. To use it,
you'd do something like:

    import scalacheck._
    import Arbitrary.arbitrary

    Gen.function1[Int, Int](arbitrary[Int])
    // returns a Gen[Int => Int] instance
    // uses implicit Cogen[Int]

The only step that is left (besides writing tests) is to start
generating new Arbitrary[_] instances that use these methods to
provide better arbitrary functions.
This commit updates the code-generation script to use the
Gen.functionN methods to produce better arbitrary functions.

This is a breaking API change. We probably need to add more
Cogen instances for end-users to avoid breaking their tests.
However, Scalacheck's own tests still pass.

At this point the feature is added, and what is needed is
clean up and fleshing out the stable of Cogen instances.
@non non force-pushed the topic/immutable-rng branch from 509c349 to b2b5759 Compare June 16, 2015 17:53
@non
Copy link
Contributor Author

non commented Jun 16, 2015

Alright, I just did what you suggested: I rebased against upstream/master and edited build.sbt.

I'm still working on the issue that @xuwei-k noticed with Arbitrary[Boolean => Boolean]. I'll push a commit and comment here once I have that issue fixed.

non added 2 commits June 17, 2015 16:53
This commit swaps out the 64-bit linear-congruent RNG for a
variant on Bob Jenkin's simple RNG [1]. It provides much
better support for the kind of reseeding that Cogen needs.

[1] http://burtleburtle.net/bob/rand/smallprng.html
This commit adds properties to ensure that Cogen + Gen
are producing all the possible functions for types.
It ensures that our RNG is doing a good job using
the entropy from function arguments to reseed itself
to produce novel output values.
@non
Copy link
Contributor Author

non commented Jun 17, 2015

These last commits fix the issue that @xuwei-k found. At this point I consider the PR feature complete.

@rickynils is there anything else you'd like me to do? Scalacheck probably needs documentation explaining how Cogen[A] works but I wasn't sure if you wanted me to tackle that in this PR or not.

@xuwei-k
Copy link
Contributor

xuwei-k commented Jun 20, 2015

👍

rickynils added a commit that referenced this pull request Jun 21, 2015
Support non-constant arbitrary functions
@rickynils rickynils merged commit ea2bbe6 into typelevel:master Jun 21, 2015
@jedesah
Copy link
Contributor

jedesah commented Jun 21, 2015

This is awesome!

@rickynils
Copy link
Contributor

Now that snapshots are published automatically, you can try out this in 1.13.0-ea2bbe6-SNAPSHOT

@carlpaten
Copy link

I found this page while looking for documentation on the topic of Cogen. I don't think there is such a thing at the moment, but you can check this Haskell blog post to get the gist of it.

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.

5 participants