From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Nathan Wagner <nw+pg(at)hydaspes(dot)if(dot)org> |
Cc: | pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: patch for geqo tweaks |
Date: | 2015-11-06 16:45:38 |
Message-ID: | 20851.1446828338@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Nathan Wagner <nw+pg(at)hydaspes(dot)if(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2015-11-06 16:52:35 | Re: Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby |
Previous Message | Robert Haas | 2015-11-06 16:42:56 | Re: Re: BUG #13685: Archiving while idle every archive_timeout with wal_level hot_standby |