Provide `int32` and `int64` equivalent functions of `int` and `float`
I could not find a way in QCheck to generate an int32 or int64 within a range, similar to range_int/-- for int and range_float/--. for float.
Is it currently possible? Otherwise, what do you think about adding those?
I guess this would also apply to functions like xxx_bound, pos_xxx, neg_xxx?
Arguably this would not be useful for the small_xxx family of functions as one could just generate int and convert them, but it may be more consistent to add those too (I don't have any opinion)
While this hasn't impacted me yet, I see that even float doesn't always have an equivalent function of int either
This is a messy issue, talking about int64, int32, float... Sorry :sweat_smile:
It would be useful, but I have little time to work on qcheck currently. I'd review/merge PRs of course :).
Thank you @c-cube
I just want to clarify that I am not coming with my Santa list and waiting for you to implement it all :sweat_smile: I'm first and foremost opening these issues to discuss adding such features to QCheck (and sometimes discover it already (partially) existed, like the runner error messages :grin: )
If/once you agree on a design, anyone (you, me, my teammates, or anyone reading this, in no particular order :grin: ) can contribute said feature. I hope I will be able to contribute some (modest) ones (or propose a rough draft when I have already implemented it for my needs, like the generator that retries in #104 )
I agree such generators would be a nice addition (also for consistency).
It should be relatively straightforward to add them if you study the existing int implementations in, e.g.,
https://github.com/c-cube/qcheck/blob/master/src/core/QCheck.ml#L158
Previously I've contributed a couple of such extensions as I required them myself. As such I see QCheck developing very much "by need" 🙂 If nothing else opening an issue is valuable to document the shortcoming to others 🤷♂️
I just want to clarify that I am not coming with my Santa list and waiting for you to implement it all sweat_smile
good, because it's march, there's still > 9 months to go :wink: .
As such I see QCheck developing very much "by need" slightly_smiling_face
Yes, that's how I see it too. This way it's sustainable and only useful stuff should be added…
It should be relatively straightforward to add them if you study the existing int implementations in, e.g.,
It is not 100% straightforward because some inner function uses Random.State.bytes which generates an int. Besides that, this looked pretty straightforward indeed.
If one has an idea how to circumvent this, maybe I can give it a shot
For, e.g., i64_bound and i64_range it should be possible to mimic the int versions, shouldn't it?
I can see pint uses RS.bits by glueing chunks of 30 random bits together. I believe we inherited that approach from https://github.com/ocaml/ocaml/blob/trunk/stdlib/random.ml#L112
In November I had my fingers in int_bound and int_range. See https://github.com/c-cube/qcheck/issues/100 (also for an explanation of the current version - and a few pitfalls)
While working on integrated shrinking I think I noticed something wrong regarding Gen.ui32/64: they are signed :thinking:
Gen.generate ~n:10 (Gen.ui32) ;;
- : int32 list =
[-573812011l; -1494688812l; -1223342577l; 732077606l; 1865555234l; 141484367l;
172385682l; 558309696l; 83912486l; -574950022l]
It is all the more surprising that the arbitraries int32/64 rely on those "unsigned" generators...
Have I missed something? I expected "unsigned" to mean "positive"
Indeed as the current implementation generates 32- or 64-bits random strings, the sign bit is random too, hence negative numbers.
As @jmid hinted I think we can generalize the current implementation of pint by taking as parameter a bit size.
Then all of positive, negative, and signed numbers (int/int32/int64) could reuse this single generator by passing either 32, 64 or Sys.int_size (minus 1 for positive- or negative-only numbers).
As the current PR https://github.com/c-cube/qcheck/pull/109 touches a lot of code, I think I won't touch this for now, but this would be great to clarify this afterwards
Edit: I wonder if the "unsigned" thingy for generators comes from the fact that Int32/64.of_string functions read "unsigned" strings? But the resulting int32/64 number is indeed signed, so this is confusing...
@Sir4ur0n Surprising indeed! Also see #90 - I can see I'm assigned but I unfortunately have my hands full in a new job.
:facepalm: I didn't see this issue, my bad, thank you :sweat_smile: