Re: patch for geqo tweaks - Mailing list pgsql-hackers

From Tom Lane
Subject Re: patch for geqo tweaks
Date
Msg-id 20851.1446828338@sss.pgh.pa.us
Whole thread Raw
In response to Re: patch for geqo tweaks  (Nathan Wagner <nw+pg@hydaspes.if.org>)
Responses Re: patch for geqo tweaks  (Nathan Wagner <nw+pg@hydaspes.if.org>)
List pgsql-hackers
Nathan Wagner <nw+pg@hydaspes.if.org> writes:
> On Wed, Nov 04, 2015 at 12:51:52PM -0500, Tom Lane wrote:
>> I'm not very impressed with the first patch: it might save a few
>> geqo_randint() calls, but it seems to do so at the price of making the
>> swap choices less random --- for instance it sure looks to me like the
>> last array element is now less likely to participate in swaps than
>> other elements.  Unless you can prove that actually the swapping is
>> still unbiased, I'm inclined to reject this part.

> If I have understood the original code correctly, we need to select two
> different random integers between 0 and num_gene-1, inclusive.  That
> happens to be num_gene possible results.

> Having chosen the first one, which I will call "swap1", we now only have
> num_gene-1 possible results, which need to range from either 0 to
> swap1-1 or from swap1+1 to num_gene-1, which is num_gene-1 possible
> results.  I treat this as a single range from 0 to num_gene-2 and
> generate a number within that range, which I will call "swap2".

> If swap2 is between 0 and swap1-1, it is in the first range, and no
> adjustment is necessary.  If it is greater than or equal to swap1, then
> it is in the second range.  However the generated swap2 in the second
> range will be between swap1 and num_gene-2, whereas we need it to be
> between swap1+1 and num_gene-1, so I add one to swap2, adjusting the
> range to the needed range.

Ah, after thinking some more, I see how that works.  I tend to think
that your other proposal of
    swap1 = geqo_randint(root, num_gene - 1, 0);    swap2 = geqo_randint(root, num_gene - 2, 0);    if (swap2 ===
swap1)       swap2 = num_gene - 1;
 

would be clearer, since only the forbidden case gets remapped.

However, really the whole argument is moot, because I notice that
geqo_mutation() is only called in the "#ifdef CX" code path, which
we don't use.  So there's little point in improving it.

(There's a fair amount of dead code in /geqo/, which I've never had
the energy to clean up, but maybe we should do that sometime.  It
seems unlikely that anyone will ever be interested in experimenting
with the ifdef'ed-out code paths.)
        regards, tom lane



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby
Next
From: Andres Freund
Date:
Subject: Re: Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby