[Issue 7067] std.random.RandomSample and RandomCover are poorly designed

[d-bugmail at puremagic.com]

http://d.puremagic.com/issues/show_bug.cgi?id=7067


Joseph Rushton Wakeling <joseph.wakeling at webdrake.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |joseph.wakeling at webdrake.ne
                   |                            |t


--- Comment #14 from Joseph Rushton Wakeling <joseph.wakeling at webdrake.net> 2013-06-20 16:02:07 PDT ---
I just want to put on record some remarks about short- and long-term solutions
to this problem.

Long-term as I think we all agree we need RNGs to be reference types.  In this
case the general design of ranges like RandomCover and RandomSample will be not
entirely different from what RandomCover has now: the constructor will take a
RNG as one of its arguments, and (as it's a reference type) this can be copied
(by reference!) to an internal variable:

    struct SomeRandomRange(Rng)
    {
        private Rng _gen;
        ...
        this(/*stuff*/, Rng gen)
        {
            _gen = gen;
            ...
        }
        ...
    }

... and then you'd have some helper functions that would handle the case where
the user doesn't specify an RNG by passing rndGen to the constructor.

However, while we're stuck with the situation we have, the design of
RandomCover is statistically completely unsafe, as you'll inevitably get
unwanted correlations due to storing the RNG by value.  As RandomCover's
constructor insists on receiving an RNG, the only way to escape this is to pass
it a new RNG seeded with unpredictableSeed.

What we can do, though, is to salvage things minimally by patching RandomCover
to use the same technique as RandomSample -- that is, if the constructor gets
passed an RNG, copy it; but if not -- call rndGen directly.  I'm going to
prepare some patches to that effect, which will also try and do something about
the .save methods of Random{Cover,Sample} -- both of which are currently
broken.

It's putting a sticking plaster on a gaping wound, but at least it should mean
that the simplest case available to the user -- doing something like this:

    auto c = randomCover(input);

... will be statistically reliable.

The caveat here is that we have to remember to tweak the constructors back to
insisting on getting an RNG once we can guarantee that RNGs will behave as
reference types.  (Is there any kind of template constraint that could be used
to guarantee reference copying semantics?)

Anyway, I'll get on with writing the patches.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------