std.random: No equality checks for doc example unittests

[kinke]

No description provided.

[dlang-bot]

Thanks for your pull request, @kinke! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.

Some tips to help speed things up:

• smaller, focused PRs are easier to review than big ones

• try not to mix up refactoring or style changes with bug fixes or feature enhancements

• provide helpful commit messages explaining the rationale behind each change

Bear in mind that large or tricky changes may require multiple rounds of review and revision.

Please see CONTRIBUTING.md for more information.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

[PetarKirov]

Perhaps we should make the assertion conditional on the size of real?

On the other hand I'm not sure why the documentation example assumes that Random is an always an alias for Mt19937 as before that, readers are left with the expectation that the alias is implementation defined and that perhaps one shouldn't rely on it if they're writing portable code:

The standard library provides an alias $(D_PARAM Random) for
whichever generator it considers the most fit for the target
environment.

@WebDrake please advice.

[kinke]

Even leaving the alias thing aside, I think checking for exact FP equality here is a bad idea (and no need for reals - just use double for portability; the rhs is a double too btw).

[WebDrake]

It does indeed seem dodgy to assume that Random is Mt19937, although right now the assumption holds true for all supported architectures. Note that this affects all the equality checks for variates in this unittest example, not just the real-valued one.

TBH I think the problems of this unittest come out of a confused purpose: it's really a module-level documentation example, so there's no need for it to check these result values at all. Checks on the results of deterministically seeded RNGs can be (and are) made in the more in-depth checks of those RNGs themselves.

I'd suggest just cutting all the equality checks on results in this unittest block. If not I guess approxEqual will suffice: feqrel is much better for comparing floating-point results, but is harder for the reader to understand, which in this context is probably more important.

[PetarKirov]

Testing exact equality of floating-point values in the documentation of this module is not a good idea. It can be either non-portable (wrong on some platforms) or correct, but requiring too much effort than necessary for something that ought to be simple in which case distracting the users from the main objective of the example.

[PetarKirov]

Given @WebDrake advice, I think it's better to simply remove the equality comparisons and instead assert that the numbers are in the expected intervals.

[WebDrake]

@ZombineDev I take it you're talking here only about the floating-point number? The integer variates generated elsewhere in the unittest should be exactly equal, assuming that Random is reliably Mt19937.

[PetarKirov]

Yes, I mean the integer cases too, since I don't think knowledge of the Random's implementation should be assumed for the readers of the first example in the documentation. Testing the exact values should be done only when the exact random engine is explicitly specified, in addition to its seed, IMO.

[WebDrake]

Oh, so you mean in this case you'd check that the resulting number is >= 0 and < 100, and for the uniform(0, 15, rnd) call you'd check that it's >= 0 and < 15?

That sounds reasonable, yes. Note that for the uniform!uint case this won't really work since the range of possible values covers the entire range of possible values for that type; but you don't really have to validate that result at all anyway.

[kinke]

Done.