Generate Rational numbers with smaller denominators

[Bodigrim]

Closes #295, making maximal denominator dependent on size instead of being fixed to 9999999999999.

> sample (arbitrarySizedFractional :: Gen Rational)
0 % 1
(-1) % 1
3 % 4
4 % 3
(-1) % 1
8 % 5
1 % 1
(-3) % 2
4 % 7
(-2) % 3
(-1) % 1

[cartazio]

Andrew: what do you think of the hybrid mixture in Olegs alternative Pr? I think both have some good bits

On Sat, May 16, 2020 at 3:17 PM Bodigrim notifications@github.com wrote:

@Bodigrim commented on this pull request.

In Test/QuickCheck/Arbitrary.hs https://github.com/nick8325/quickcheck/pull/296#discussion_r426182506:

@@ -998,13 +998,10 @@ inBounds fi g = fmap fi (g suchThat (\x -> toInteger x == toInteger (fi x))) -- and its maximum absolute value depends on the size parameter. arbitrarySizedFractional :: Fractional a => Gen a arbitrarySizedFractional =

• sized $ \n ->
• let n' = toInteger n in
• do b <- chooseInteger (1, precision)
• a <- chooseInteger ((-n') b, n' b)
• return (fromRational (a % b))
• where
• precision = 9999999999999 :: Integer
• sized $ \n -> do
• numer <- chooseInt (-n, n)
• denom <- chooseInt (1, max 1 n)

Err, I'm not sure what facts about half-integers can be derived from this plot (and what distribution you desire to obtain). As quoted above, sample generates at least some half-integers, on contrary to the existing implementation and to your suggestion.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/nick8325/quickcheck/pull/296#discussion_r426182506, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABBQQ3EBGLEESIHZTLSDTRR3RDXANCNFSM4NC6TXMA .

[Bodigrim]

As discussed above, @phadej and I are tackling different aspects of the issue. However, after some consideration, I came to a conclusion that it is futile to improve arbitrarySizedFractional: it does not make much sense for Fractionals other than Double or Rational. E. g., it returns unbounded by size results for modular arithmetic or Galois fields. And given that #297 does a very good job for Double, the only thing left is Rational. That said, I decided to repurpose this PR to improving instance Arbitrary (Ratio a).

[Bodigrim]

@nick8325 just a gentle reminder about this PR.

[Bodigrim]

@nick8325 ping

[Bodigrim]

@nick8325 ping

[nick8325]

Sorry about the unreasonably-long delay, but it's merged now. Thanks for the patch!

I tweaked the generator to do this:

  sized $ \n -> do
    denom <- chooseInt (1, max 1 n)
    numer <- chooseInt (-n*denom, n*denom)
    pure $ fromIntegral numer / fromIntegral denom

so that the generated numbers are drawn roughly uniformly from the interval -n...n, and have a maximum denominator of n. That is, increasing the size increases both the magnitude and the maximum denominator. I hope that's reasonable but let me know if you see some problem with it.

[Bodigrim]

@nick8325 that's probably fine, thanks.