Return-Path: 
 <dev-return-150507-apmail-commons-dev-archive=commons.apache.org@commons.apache.org>
X-Original-To: apmail-commons-dev-archive@www.apache.org
Delivered-To: apmail-commons-dev-archive@www.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 8D98817A5B
	for <apmail-commons-dev-archive@www.apache.org>;
 Thu,  9 Oct 2014 12:44:26 +0000 (UTC)
Received: (qmail 56468 invoked by uid 500); 9 Oct 2014 12:44:26 -0000
Delivered-To: apmail-commons-dev-archive@commons.apache.org
Received: (qmail 56349 invoked by uid 500); 9 Oct 2014 12:44:26 -0000
Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:dev-help@commons.apache.org>
List-Unsubscribe: <mailto:dev-unsubscribe@commons.apache.org>
List-Post: <mailto:dev@commons.apache.org>
List-Id: <dev.commons.apache.org>
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Delivered-To: mailing list dev@commons.apache.org
Received: (qmail 56337 invoked by uid 99); 9 Oct 2014 12:44:25 -0000
Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230)
    by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 09 Oct 2014 12:44:25 +0000
X-ASF-Spam-Status: No, hits=-0.7 required=5.0
	tests=RCVD_IN_DNSWL_LOW,SPF_PASS
X-Spam-Check-By: apache.org
Received-SPF: pass (nike.apache.org: local policy)
Received: from [193.74.71.28] (HELO sif.is.scarlet.be) (193.74.71.28)
    by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 09 Oct 2014 12:43:58 +0000
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=scarlet.be;
	s=scarlet; t=1412858636;
	bh=ptk+au4sUnaTvXkaB0yUb3SsbJL8xq/++NajEj1y03Q=;
	h=MIME-Version:Content-Type:Content-Transfer-Encoding:Date:From:To:
	 Subject:In-Reply-To:References:Message-ID;
	b=U9s5i/rK0SUAVQm/gy0iy7Ly/6bHB/TRFrplrmip26wdBxX51Ez88u8B1aiZrCTVU
	 CuEnB74Ia/a8o7BdM9e5vnTF9Dt496xp+SuKNeEITzdR9g08n9WUmVRtwbU0B8s5Bg
	 0SwVyIUepN+m89fADqhbibG3GsyLGME5ohL2S9d4=
Received: from webmail.scarlet.be (gresham.is.scarlet.be [193.74.71.215])
	by sif.is.scarlet.be (8.14.5/8.14.5) with ESMTP id s99ChuJM024799
	for <dev@commons.apache.org>; Thu, 9 Oct 2014 14:43:56 +0200
X-Scarlet: d=1412858636 c=193.74.71.215
Received: from pno-astro-26.ulb.ac.be ([164.15.138.26])
 via astropc12.ulb.ac.be ([164.15.138.26])
 by webmail.scarlet.be
 with HTTP (HTTP/1.1 POST); Thu, 09 Oct 2014 14:43:56 +0200
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8;
 format=flowed
Content-Transfer-Encoding: 7bit
Date: Thu, 09 Oct 2014 14:43:56 +0200
From: Gilles <gilles@harfang.homelinux.org>
To: Commons Developers List <dev@commons.apache.org>
Subject: Re: [math] Thread-safety and initialization overhead in
 distribution sample() methods
In-Reply-To: 
 <CAJSjvwvVjbURn8W76RWdimyOa8zte=73s5s9nYkq17_GEXTQzA@mail.gmail.com>
References: <5434878A.1010406@gmail.com> <543529EE.4020708@spaceroots.org>
 <CAJSjvwv4DvSFumqd0k5O4=NAqiN6J5zmk9A9v77skhh91FTVCQ@mail.gmail.com>
 <54355CF2.7020204@gmail.com> <ca0ae9095737542da5c796a161c60c0d@scarlet.be>
 <5435945A.6080306@spaceroots.org>
 <cecd34cc5a2a973e113937629a415044@scarlet.be>
 <CAJSjvwvVjbURn8W76RWdimyOa8zte=73s5s9nYkq17_GEXTQzA@mail.gmail.com>
Message-ID: <ad96ee0b8b1e5ac8ab3467b6260448dd@scarlet.be>
X-Sender: gilles@harfang.homelinux.org
User-Agent: Scarlet Webmail
X-DCC-scarlet.be-Metrics: sif; whitelist
X-Virus-Scanned: clamav-milter 0.98.1-exp at sif
X-Virus-Status: Clean
X-Virus-Checked: Checked by ClamAV on apache.org

> > [...]
>>>
>>
>> That proposal would
>> 1. fix the performance issue reported in MATH-1154,
>>
>
> this proposal would only fix it in 4.0, as we can not remove the rng
> references in the distribution classes immediately.

<tease>
  There is not a single way
  I can oppose what you say:
  This cannot be a good day!
</tease>

The proposal is design-oriented.
I seem to recall (but I may be wrong) that a similar thing was
proposed (not by me) when "sample" was introduced, but was not
retained.
It was a wrong decision at the time, since this would have made
the design cleaner. But this argument is generally not well
received here, unfortunately (even when the opportunity for
change is given by an identified issue).

>
>> 2. fix that issue without resorting to the so called "smelly
>>    workaround",
>>
>
> there is nothing smelly about it,

I agree: That was not _my_ statement.

> and if you want to fix this issue in 3.4
> there is not much choice

As I said early on, you could have applied the patch already. I
do not even know why there need to be a discussion about it as
it fixes an issue with zero side-effect.

> (apart from the lazy initialization proposal).

Otmar's proposal (explicit "LazyRNG"): +1
Hidden lazy init in "AbstractRealDistribution" or in the
individual RNGs: -1 (with rationale given in a previous message).

But I now agree with Phil that it is more complex than necessary.
(i.e. I humbly find my proposal simpler).

>> 3. simplify the "RealDistribution" interface namely by
>>    removing the "reseedRandomGenerator" method (whose presence
>>    has always been a flaw, IMO): the RNG is and stays under the
>>    caller's explicit responsibility,
>> 4. simplify the concrete distribution classes, by removing all
>>    the constructors that take a RNG argument,
>> 5. make the distribution classes thread-safe, by making the
>>    "devilishly hard" problem (of ensuring RNG thread-safety) go
>>    away.
>> Moreover:
>> 6. An application layer that only needs sampling could pass a
>>    "Sampler" object: hiding the implicit distribution could make
>>    the application cleaner (self-documented).
>> 7. The modification can be backwards compatible, and user can
>>    upgrade easily.
>>
>
> with the other points I agree,

Thank you!

And I forgot some:

8a. No risk to raise a NPE by an unforeseen usage of "sample", and thus
8b. No need to document a warning about it.

> but I would make the sample(RandomGenerator)
> method public, so that users have the choice to directly call it 
> rather
> than create a Sampler for this distribution.

The idea (i.e my opinion, which I naturally find better, as
long as I hold it :) was to minimize the public interface of
"RealDistribution", up to not even mention the notion of
"sampling".
Actually, we could separate even more and have "Sampler" and
"SamplerProvider" interfaces, independent of the
"RealDistribution" interface.
Thus each concrete distribution is free to implement and/or
advertize (or not) a sampling functionality.

This is even cleaner; but it requires a little refactoring of the
unit tests (i.e. define a method "makeSamplerProvider" to get an
object of the apporpriate type for sampling-specific tests).

Alternatively, if we make "sample" part of "RealDistribution", we
limit the generality a priori. Even though it is not a problem
right now (as all the distributions implemented in CM can provide
sampling), we cannot know (as we didn't know that issue MATH-1154
would come up) and as simple as can be (but not simpler) is better.


Gilles


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org