Re: commons-rng git commit: Adding PoissonSampler deprecations. Use the correct faster public methods

classic Classic list List threaded Threaded
19 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: commons-rng git commit: Adding PoissonSampler deprecations. Use the correct faster public methods

Rob Tompkins-2
@Gilles - thoughts here?? Just kinda what I was thinking, but I’m only a +0 on this change. So, if you want to revert it before going up with 1.1, that’s fine.

-Rob

> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>
> Repository: commons-rng
> Updated Branches:
>  refs/heads/1.1 50b984b1d -> f8159f28a
>
>
> Adding PoissonSampler deprecations. Use the correct faster public methods
>
>
> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
> Commit: http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
> Tree: http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
> Diff: http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>
> Branch: refs/heads/1.1
> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
> Parents: 50b984b
> Author: Rob Tompkins <[hidden email]>
> Authored: Wed Aug 8 12:42:45 2018 -0400
> Committer: Rob Tompkins <[hidden email]>
> Committed: Wed Aug 8 12:42:45 2018 -0400
>
> ----------------------------------------------------------------------
> .../sampling/distribution/PoissonSampler.java   | 42 ++++++++++++++++++++
> 1 file changed, 42 insertions(+)
> ----------------------------------------------------------------------
>
>
> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
> ----------------------------------------------------------------------
> diff --git a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
> index d0733ba..29d0e4e 100644
> --- a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
> +++ b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
> @@ -67,6 +67,48 @@ public class PoissonSampler
>         return poissonSampler.sample();
>     }
>
> +    /**
> +     * @return a random value from a uniform distribution in the
> +     * interval {@code [0, 1)}.
> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
> +     * as it is considerably faster.
> +     */
> +    @Deprecated
> +    protected double nextDouble() {
> +        return super.nextDouble();
> +    }
> +
> +    /**
> +     * @return a random {@code int} value.
> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
> +     * as it is considerably faster.
> +     */
> +    @Deprecated
> +    protected int nextInt() {
> +        return super.nextInt();
> +    }
> +
> +    /**
> +     * @param max Upper bound (excluded).
> +     * @return a random {@code int} value in the interval {@code [0, max)}.
> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
> +     *      * as it is considerably faster.
> +     */
> +    @Deprecated
> +    protected int nextInt(int max) {
> +        return super.nextInt(max);
> +    }
> +
> +    /**
> +     * @return a random {@code long} value.
> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
> +     *      * as it is considerably faster.
> +     */
> +    @Deprecated
> +    protected long nextLong() {
> +        return super.nextLong();
> +    }
> +
>     /** {@inheritDoc} */
>     @Override
>     public String toString() {
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-rng git commit: Adding PoissonSampler deprecations. Use the correct faster public methods

Gilles Sadowski
Hello Rob.

On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
> only a +0 on this change. So, if you want to revert it before going
> up
> with 1.1, that’s fine.

I don't understand this change after what I answered to Gary's strange
proposal.
The comment is *wrong*.  As I said previously, the base class provides
boiler-plate code; that is *not* deprecated.  The issue is that those
methods were not meant to be used further down the hierarchy (in user's
subclasses). [And now, furthermore, they are not used anymore in class
"PoissonSampler", following the change in RNG-50 (delegation to other
classes).]
The sampler classes should have been made "final"; but now, this change
would also be "breaking" (even though I doubt about legitimate
use-cases
for inherithing from the sampler implementations).

Please revert.

Thanks,
Gilles

>
> -Rob
>
>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>
>> Repository: commons-rng
>> Updated Branches:
>>  refs/heads/1.1 50b984b1d -> f8159f28a
>>
>>
>> Adding PoissonSampler deprecations. Use the correct faster public
>> methods
>>
>>
>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>> Commit:
>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>> Tree:
>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>> Diff:
>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>
>> Branch: refs/heads/1.1
>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>> Parents: 50b984b
>> Author: Rob Tompkins <[hidden email]>
>> Authored: Wed Aug 8 12:42:45 2018 -0400
>> Committer: Rob Tompkins <[hidden email]>
>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>
>>
>> ----------------------------------------------------------------------
>> .../sampling/distribution/PoissonSampler.java   | 42
>> ++++++++++++++++++++
>> 1 file changed, 42 insertions(+)
>>
>> ----------------------------------------------------------------------
>>
>>
>>
>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>
>> ----------------------------------------------------------------------
>> diff --git
>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>> index d0733ba..29d0e4e 100644
>> ---
>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>> +++
>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>         return poissonSampler.sample();
>>     }
>>
>> +    /**
>> +     * @return a random value from a uniform distribution in the
>> +     * interval {@code [0, 1)}.
>> +     * @deprecated - one should be using the {@link
>> PoissonSampler#sample()} method,
>> +     * as it is considerably faster.
>> +     */
>> +    @Deprecated
>> +    protected double nextDouble() {
>> +        return super.nextDouble();
>> +    }
>> +
>> +    /**
>> +     * @return a random {@code int} value.
>> +     * @deprecated - one should be using the {@link
>> PoissonSampler#sample()} method,
>> +     * as it is considerably faster.
>> +     */
>> +    @Deprecated
>> +    protected int nextInt() {
>> +        return super.nextInt();
>> +    }
>> +
>> +    /**
>> +     * @param max Upper bound (excluded).
>> +     * @return a random {@code int} value in the interval {@code
>> [0, max)}.
>> +     * @deprecated - one should be using the {@link
>> PoissonSampler#sample()} method,
>> +     *      * as it is considerably faster.
>> +     */
>> +    @Deprecated
>> +    protected int nextInt(int max) {
>> +        return super.nextInt(max);
>> +    }
>> +
>> +    /**
>> +     * @return a random {@code long} value.
>> +     * @deprecated - one should be using the {@link
>> PoissonSampler#sample()} method,
>> +     *      * as it is considerably faster.
>> +     */
>> +    @Deprecated
>> +    protected long nextLong() {
>> +        return super.nextLong();
>> +    }
>> +
>>     /** {@inheritDoc} */
>>     @Override
>>     public String toString() {
>>



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

Gilles Sadowski
Hi all.

On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:

> Hello Rob.
>
> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>> only a +0 on this change. So, if you want to revert it before going
>> up
>> with 1.1, that’s fine.
>
> I don't understand this change after what I answered to Gary's
> strange
> proposal.
> The comment is *wrong*.  As I said previously, the base class
> provides
> boiler-plate code; that is *not* deprecated.  The issue is that those
> methods were not meant to be used further down the hierarchy (in
> user's
> subclasses). [And now, furthermore, they are not used anymore in
> class
> "PoissonSampler", following the change in RNG-50 (delegation to other
> classes).]
> The sampler classes should have been made "final"; but now, this
> change
> would also be "breaking" (even though I doubt about legitimate
> use-cases
> for inherithing from the sampler implementations).

Assuming it's unlikely that application developers would have
  * called protected methods of "SamplerBase",[1]
  * create subclasses of "SamplerBase",[1]
  * create subclasses of the sampler classes,[1]
I suggest to
  * make "SamplerBase" package private
  * make the sampler clases "final".

The above are the *less* intrusive fixes, as they would only
potentially impact application developers by making them aware
that they have made incorrect usage of an *inadvertently*
public API.  Correct usage will not be impacted even though
the change is not BC.[2]

Any objection to that fix?[3]

Regards,
Gilles

[1] Rationale: (a) There is no reason to do that and
                (b) "Commons RNG" isn't much used at this point:
       
http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
[2] I recall that a similar situation (BC breaking in a minor
     release) occurred in CM, at a time where the number of
     potential users was much larger.
[3] I'll add a prominent warning in the changelog to the effect
     that people wanting to continue with incorrect usage of the
     samplers should not upgrade. ;-)

>
> Please revert.
>
> Thanks,
> Gilles
>
>>
>> -Rob
>>
>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>
>>> Repository: commons-rng
>>> Updated Branches:
>>>  refs/heads/1.1 50b984b1d -> f8159f28a
>>>
>>>
>>> Adding PoissonSampler deprecations. Use the correct faster public
>>> methods
>>>
>>>
>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>> Commit:
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>> Tree:
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>> Diff:
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>
>>> Branch: refs/heads/1.1
>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>> Parents: 50b984b
>>> Author: Rob Tompkins <[hidden email]>
>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>> Committer: Rob Tompkins <[hidden email]>
>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>
>>>
>>> ----------------------------------------------------------------------
>>> .../sampling/distribution/PoissonSampler.java   | 42
>>> ++++++++++++++++++++
>>> 1 file changed, 42 insertions(+)
>>>
>>> ----------------------------------------------------------------------
>>>
>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>
>>> ----------------------------------------------------------------------
>>> diff --git
>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> index d0733ba..29d0e4e 100644
>>> ---
>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> +++
>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>         return poissonSampler.sample();
>>>     }
>>>
>>> +    /**
>>> +     * @return a random value from a uniform distribution in the
>>> +     * interval {@code [0, 1)}.
>>> +     * @deprecated - one should be using the {@link
>>> PoissonSampler#sample()} method,
>>> +     * as it is considerably faster.
>>> +     */
>>> +    @Deprecated
>>> +    protected double nextDouble() {
>>> +        return super.nextDouble();
>>> +    }
>>> +
>>> +    /**
>>> +     * @return a random {@code int} value.
>>> +     * @deprecated - one should be using the {@link
>>> PoissonSampler#sample()} method,
>>> +     * as it is considerably faster.
>>> +     */
>>> +    @Deprecated
>>> +    protected int nextInt() {
>>> +        return super.nextInt();
>>> +    }
>>> +
>>> +    /**
>>> +     * @param max Upper bound (excluded).
>>> +     * @return a random {@code int} value in the interval {@code
>>> [0, max)}.
>>> +     * @deprecated - one should be using the {@link
>>> PoissonSampler#sample()} method,
>>> +     *      * as it is considerably faster.
>>> +     */
>>> +    @Deprecated
>>> +    protected int nextInt(int max) {
>>> +        return super.nextInt(max);
>>> +    }
>>> +
>>> +    /**
>>> +     * @return a random {@code long} value.
>>> +     * @deprecated - one should be using the {@link
>>> PoissonSampler#sample()} method,
>>> +     *      * as it is considerably faster.
>>> +     */
>>> +    @Deprecated
>>> +    protected long nextLong() {
>>> +        return super.nextLong();
>>> +    }
>>> +
>>>     /** {@inheritDoc} */
>>>     @Override
>>>     public String toString() {
>>>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

Gilles Sadowski
On Thu, 09 Aug 2018 13:02:15 +0200, Gilles wrote:

> Hi all.
>
> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
>> Hello Rob.
>>
>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>>> only a +0 on this change. So, if you want to revert it before going
>>> up
>>> with 1.1, that’s fine.
>>
>> I don't understand this change after what I answered to Gary's
>> strange
>> proposal.
>> The comment is *wrong*.  As I said previously, the base class
>> provides
>> boiler-plate code; that is *not* deprecated.  The issue is that
>> those
>> methods were not meant to be used further down the hierarchy (in
>> user's
>> subclasses). [And now, furthermore, they are not used anymore in
>> class
>> "PoissonSampler", following the change in RNG-50 (delegation to
>> other
>> classes).]
>> The sampler classes should have been made "final"; but now, this
>> change
>> would also be "breaking" (even though I doubt about legitimate
>> use-cases
>> for inherithing from the sampler implementations).
>
> Assuming it's unlikely that application developers would have
>  * called protected methods of "SamplerBase",[1]
>  * create subclasses of "SamplerBase",[1]
>  * create subclasses of the sampler classes,[1]
> I suggest to
>  * make "SamplerBase" package private
>  * make the sampler clases "final".
>
> The above are the *less* intrusive fixes, as they would only
> potentially impact application developers by making them aware
> that they have made incorrect usage of an *inadvertently*
> public API.  Correct usage will not be impacted even though
> the change is not BC.[2]
>
> Any objection to that fix?[3]
>
> Regards,
> Gilles
>
> [1] Rationale: (a) There is no reason to do that and
>                (b) "Commons RNG" isn't much used at this point:
>
>
> http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages

Additional data points: Those projects use
   org.apache.commons.rng.sampling.CollectionSampler
   org.apache.commons.rng.sampling.ListSampler
   org.apache.commons.rng.sampling.PermutationSampler
   org.apache.commons.rng.sampling.distribution.ContinuousUniformSampler
   org.apache.commons.rng.sampling.distribution.DiscreteUniformSampler

Only the latter two imports are related to this issue and usage
seems OK (AFAICT since it's written in "Clojure"...)

Gilles

> [2] I recall that a similar situation (BC breaking in a minor
>     release) occurred in CM, at a time where the number of
>     potential users was much larger.
> [3] I'll add a prominent warning in the changelog to the effect
>     that people wanting to continue with incorrect usage of the
>     samplers should not upgrade. ;-)
>
>>
>> Please revert.
>>
>> Thanks,
>> Gilles
>>
>>>
>>> -Rob
>>>
>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>>
>>>> Repository: commons-rng
>>>> Updated Branches:
>>>>  refs/heads/1.1 50b984b1d -> f8159f28a
>>>>
>>>>
>>>> Adding PoissonSampler deprecations. Use the correct faster public
>>>> methods
>>>>
>>>>
>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>> Commit:
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>> Tree:
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>> Diff:
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>>
>>>> Branch: refs/heads/1.1
>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>> Parents: 50b984b
>>>> Author: Rob Tompkins <[hidden email]>
>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>> Committer: Rob Tompkins <[hidden email]>
>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>> .../sampling/distribution/PoissonSampler.java   | 42
>>>> ++++++++++++++++++++
>>>> 1 file changed, 42 insertions(+)
>>>>
>>>> ----------------------------------------------------------------------
>>>>
>>>>
>>>>
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>
>>>> ----------------------------------------------------------------------
>>>> diff --git
>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> index d0733ba..29d0e4e 100644
>>>> ---
>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> +++
>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>         return poissonSampler.sample();
>>>>     }
>>>>
>>>> +    /**
>>>> +     * @return a random value from a uniform distribution in the
>>>> +     * interval {@code [0, 1)}.
>>>> +     * @deprecated - one should be using the {@link
>>>> PoissonSampler#sample()} method,
>>>> +     * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected double nextDouble() {
>>>> +        return super.nextDouble();
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * @return a random {@code int} value.
>>>> +     * @deprecated - one should be using the {@link
>>>> PoissonSampler#sample()} method,
>>>> +     * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected int nextInt() {
>>>> +        return super.nextInt();
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * @param max Upper bound (excluded).
>>>> +     * @return a random {@code int} value in the interval {@code
>>>> [0, max)}.
>>>> +     * @deprecated - one should be using the {@link
>>>> PoissonSampler#sample()} method,
>>>> +     *      * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected int nextInt(int max) {
>>>> +        return super.nextInt(max);
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * @return a random {@code long} value.
>>>> +     * @deprecated - one should be using the {@link
>>>> PoissonSampler#sample()} method,
>>>> +     *      * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected long nextLong() {
>>>> +        return super.nextLong();
>>>> +    }
>>>> +
>>>>     /** {@inheritDoc} */
>>>>     @Override
>>>>     public String toString() {
>>>>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

Rob Tompkins
In reply to this post by Gilles Sadowski
Are we instead trying to remove the “extends” from all of those classes?

My thought was to override the method and javadoc locally (in the actual subclass) so that we can give reasonable deprecation messages, but that’s only a thought. I’m ok with whatever.

-Rob

> On Aug 9, 2018, at 7:02 AM, Gilles <[hidden email]> wrote:
>
> Hi all.
>
> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
>> Hello Rob.
>>
>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>>> only a +0 on this change. So, if you want to revert it before going up
>>> with 1.1, that’s fine.
>>
>> I don't understand this change after what I answered to Gary's strange
>> proposal.
>> The comment is *wrong*.  As I said previously, the base class provides
>> boiler-plate code; that is *not* deprecated.  The issue is that those
>> methods were not meant to be used further down the hierarchy (in user's
>> subclasses). [And now, furthermore, they are not used anymore in class
>> "PoissonSampler", following the change in RNG-50 (delegation to other
>> classes).]
>> The sampler classes should have been made "final"; but now, this change
>> would also be "breaking" (even though I doubt about legitimate use-cases
>> for inherithing from the sampler implementations).
>
> Assuming it's unlikely that application developers would have
> * called protected methods of "SamplerBase",[1]
> * create subclasses of "SamplerBase",[1]
> * create subclasses of the sampler classes,[1]
> I suggest to
> * make "SamplerBase" package private
> * make the sampler clases "final".
>
> The above are the *less* intrusive fixes, as they would only
> potentially impact application developers by making them aware
> that they have made incorrect usage of an *inadvertently*
> public API.  Correct usage will not be impacted even though
> the change is not BC.[2]
>
> Any objection to that fix?[3]
>
> Regards,
> Gilles
>
> [1] Rationale: (a) There is no reason to do that and
>               (b) "Commons RNG" isn't much used at this point:
>      http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
> [2] I recall that a similar situation (BC breaking in a minor
>    release) occurred in CM, at a time where the number of
>    potential users was much larger.
> [3] I'll add a prominent warning in the changelog to the effect
>    that people wanting to continue with incorrect usage of the
>    samplers should not upgrade. ;-)
>
>>
>> Please revert.
>>
>> Thanks,
>> Gilles
>>
>>>
>>> -Rob
>>>
>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>>
>>>> Repository: commons-rng
>>>> Updated Branches:
>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>>
>>>>
>>>> Adding PoissonSampler deprecations. Use the correct faster public methods
>>>>
>>>>
>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>> Commit: http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>>
>>>> Branch: refs/heads/1.1
>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>> Parents: 50b984b
>>>> Author: Rob Tompkins <[hidden email]>
>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>> Committer: Rob Tompkins <[hidden email]>
>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>>
>>>> ----------------------------------------------------------------------
>>>> .../sampling/distribution/PoissonSampler.java   | 42 ++++++++++++++++++++
>>>> 1 file changed, 42 insertions(+)
>>>> ----------------------------------------------------------------------
>>>>
>>>>
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> ----------------------------------------------------------------------
>>>> diff --git a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> index d0733ba..29d0e4e 100644
>>>> --- a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> +++ b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>        return poissonSampler.sample();
>>>>    }
>>>>
>>>> +    /**
>>>> +     * @return a random value from a uniform distribution in the
>>>> +     * interval {@code [0, 1)}.
>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>> +     * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected double nextDouble() {
>>>> +        return super.nextDouble();
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * @return a random {@code int} value.
>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>> +     * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected int nextInt() {
>>>> +        return super.nextInt();
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * @param max Upper bound (excluded).
>>>> +     * @return a random {@code int} value in the interval {@code [0, max)}.
>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>> +     *      * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected int nextInt(int max) {
>>>> +        return super.nextInt(max);
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * @return a random {@code long} value.
>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>> +     *      * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected long nextLong() {
>>>> +        return super.nextLong();
>>>> +    }
>>>> +
>>>>    /** {@inheritDoc} */
>>>>    @Override
>>>>    public String toString() {
>>>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-rng git commit: Adding PoissonSampler deprecations. Use the correct faster public methods

Rob Tompkins
In reply to this post by Gilles Sadowski


> On Aug 8, 2018, at 1:59 PM, Gilles <[hidden email]> wrote:
>
> Hello Rob.
>
> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>> only a +0 on this change. So, if you want to revert it before going up
>> with 1.1, that’s fine.
>
> I don't understand this change after what I answered to Gary's strange
> proposal.
> The comment is *wrong*.  As I said previously, the base class provides
> boiler-plate code; that is *not* deprecated.  The issue is that those
> methods were not meant to be used further down the hierarchy (in user's
> subclasses). [And now, furthermore, they are not used anymore in class
> "PoissonSampler", following the change in RNG-50 (delegation to other
> classes).]
> The sampler classes should have been made "final"; but now, this change
> would also be "breaking" (even though I doubt about legitimate use-cases
> for inherithing from the sampler implementations).

My goal was to have the class look like is has the same functionality at 1.0, but instead accommodate for local javadoc changes
to point out what the user should be doing. That said, it’s entirely possible that I wasn’t precise enough with the “@deprecated” messaging
due to lack of familiarity with the issue at hand.

It is however unclear to me how these changes would be breaking as the methods exposed on the class don’t change signature or
accessibility quantifier, but I could have overlooked something.

Regardless, it was just an idea, and I figured the easiest way to make a proposal was to push a commit up. Like I said I’m a +0 on it, and if
we come up with a better mechanism, then I’m on board with that.

Cheers,
-Rob

>
> Please revert.
>
> Thanks,
> Gilles
>
>>
>> -Rob
>>
>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>
>>> Repository: commons-rng
>>> Updated Branches:
>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>
>>>
>>> Adding PoissonSampler deprecations. Use the correct faster public methods
>>>
>>>
>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>> Commit: http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>
>>> Branch: refs/heads/1.1
>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>> Parents: 50b984b
>>> Author: Rob Tompkins <[hidden email]>
>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>> Committer: Rob Tompkins <[hidden email]>
>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>
>>> ----------------------------------------------------------------------
>>> .../sampling/distribution/PoissonSampler.java   | 42 ++++++++++++++++++++
>>> 1 file changed, 42 insertions(+)
>>> ----------------------------------------------------------------------
>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> ----------------------------------------------------------------------
>>> diff --git a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> index d0733ba..29d0e4e 100644
>>> --- a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> +++ b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>        return poissonSampler.sample();
>>>    }
>>>
>>> +    /**
>>> +     * @return a random value from a uniform distribution in the
>>> +     * interval {@code [0, 1)}.
>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>> +     * as it is considerably faster.
>>> +     */
>>> +    @Deprecated
>>> +    protected double nextDouble() {
>>> +        return super.nextDouble();
>>> +    }
>>> +
>>> +    /**
>>> +     * @return a random {@code int} value.
>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>> +     * as it is considerably faster.
>>> +     */
>>> +    @Deprecated
>>> +    protected int nextInt() {
>>> +        return super.nextInt();
>>> +    }
>>> +
>>> +    /**
>>> +     * @param max Upper bound (excluded).
>>> +     * @return a random {@code int} value in the interval {@code [0, max)}.
>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>> +     *      * as it is considerably faster.
>>> +     */
>>> +    @Deprecated
>>> +    protected int nextInt(int max) {
>>> +        return super.nextInt(max);
>>> +    }
>>> +
>>> +    /**
>>> +     * @return a random {@code long} value.
>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>> +     *      * as it is considerably faster.
>>> +     */
>>> +    @Deprecated
>>> +    protected long nextLong() {
>>> +        return super.nextLong();
>>> +    }
>>> +
>>>    /** {@inheritDoc} */
>>>    @Override
>>>    public String toString() {
>>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email] <mailto:[hidden email]>
> For additional commands, e-mail: [hidden email] <mailto:[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

Gilles Sadowski
In reply to this post by Rob Tompkins
On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote:
> Are we instead trying to remove the “extends” from all of those
> classes?

I'm not sure what you mean, sorry.

"SamplerBase" was meant has a utility/shortcut/boiler-plate in order
to copy the "rng" argument in one place, instead of having a field
in each sampler class. It is purely internal to this library (in C++,
inheritance would have been "private").

> My thought was to override the method and javadoc locally (in the
> actual subclass) so that we can give reasonable deprecation messages,
> but that’s only a thought. I’m ok with whatever.

The message would aim at the wrong target: for developers of this
library, there is no deprecation (the boiler-plate code is there
to be used); for application developers, the class should not be
there to be used (hence: package private).

Gilles

> -Rob
>
>> On Aug 9, 2018, at 7:02 AM, Gilles <[hidden email]>
>> wrote:
>>
>> Hi all.
>>
>> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
>>> Hello Rob.
>>>
>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>>>> only a +0 on this change. So, if you want to revert it before
>>>> going up
>>>> with 1.1, that’s fine.
>>>
>>> I don't understand this change after what I answered to Gary's
>>> strange
>>> proposal.
>>> The comment is *wrong*.  As I said previously, the base class
>>> provides
>>> boiler-plate code; that is *not* deprecated.  The issue is that
>>> those
>>> methods were not meant to be used further down the hierarchy (in
>>> user's
>>> subclasses). [And now, furthermore, they are not used anymore in
>>> class
>>> "PoissonSampler", following the change in RNG-50 (delegation to
>>> other
>>> classes).]
>>> The sampler classes should have been made "final"; but now, this
>>> change
>>> would also be "breaking" (even though I doubt about legitimate
>>> use-cases
>>> for inherithing from the sampler implementations).
>>
>> Assuming it's unlikely that application developers would have
>> * called protected methods of "SamplerBase",[1]
>> * create subclasses of "SamplerBase",[1]
>> * create subclasses of the sampler classes,[1]
>> I suggest to
>> * make "SamplerBase" package private
>> * make the sampler clases "final".
>>
>> The above are the *less* intrusive fixes, as they would only
>> potentially impact application developers by making them aware
>> that they have made incorrect usage of an *inadvertently*
>> public API.  Correct usage will not be impacted even though
>> the change is not BC.[2]
>>
>> Any objection to that fix?[3]
>>
>> Regards,
>> Gilles
>>
>> [1] Rationale: (a) There is no reason to do that and
>>               (b) "Commons RNG" isn't much used at this point:
>>      
>> http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
>> [2] I recall that a similar situation (BC breaking in a minor
>>    release) occurred in CM, at a time where the number of
>>    potential users was much larger.
>> [3] I'll add a prominent warning in the changelog to the effect
>>    that people wanting to continue with incorrect usage of the
>>    samplers should not upgrade. ;-)
>>
>>>
>>> Please revert.
>>>
>>> Thanks,
>>> Gilles
>>>
>>>>
>>>> -Rob
>>>>
>>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>>>
>>>>> Repository: commons-rng
>>>>> Updated Branches:
>>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>>>
>>>>>
>>>>> Adding PoissonSampler deprecations. Use the correct faster public
>>>>> methods
>>>>>
>>>>>
>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>>> Commit:
>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>>> Tree:
>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>>> Diff:
>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>>>
>>>>> Branch: refs/heads/1.1
>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>>> Parents: 50b984b
>>>>> Author: Rob Tompkins <[hidden email]>
>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>>> Committer: Rob Tompkins <[hidden email]>
>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>>>
>>>>>
>>>>> ----------------------------------------------------------------------
>>>>> .../sampling/distribution/PoissonSampler.java   | 42
>>>>> ++++++++++++++++++++
>>>>> 1 file changed, 42 insertions(+)
>>>>>
>>>>> ----------------------------------------------------------------------
>>>>>
>>>>>
>>>>>
>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>
>>>>> ----------------------------------------------------------------------
>>>>> diff --git
>>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>> index d0733ba..29d0e4e 100644
>>>>> ---
>>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>> +++
>>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>>        return poissonSampler.sample();
>>>>>    }
>>>>>
>>>>> +    /**
>>>>> +     * @return a random value from a uniform distribution in the
>>>>> +     * interval {@code [0, 1)}.
>>>>> +     * @deprecated - one should be using the {@link
>>>>> PoissonSampler#sample()} method,
>>>>> +     * as it is considerably faster.
>>>>> +     */
>>>>> +    @Deprecated
>>>>> +    protected double nextDouble() {
>>>>> +        return super.nextDouble();
>>>>> +    }
>>>>> +
>>>>> +    /**
>>>>> +     * @return a random {@code int} value.
>>>>> +     * @deprecated - one should be using the {@link
>>>>> PoissonSampler#sample()} method,
>>>>> +     * as it is considerably faster.
>>>>> +     */
>>>>> +    @Deprecated
>>>>> +    protected int nextInt() {
>>>>> +        return super.nextInt();
>>>>> +    }
>>>>> +
>>>>> +    /**
>>>>> +     * @param max Upper bound (excluded).
>>>>> +     * @return a random {@code int} value in the interval {@code
>>>>> [0, max)}.
>>>>> +     * @deprecated - one should be using the {@link
>>>>> PoissonSampler#sample()} method,
>>>>> +     *      * as it is considerably faster.
>>>>> +     */
>>>>> +    @Deprecated
>>>>> +    protected int nextInt(int max) {
>>>>> +        return super.nextInt(max);
>>>>> +    }
>>>>> +
>>>>> +    /**
>>>>> +     * @return a random {@code long} value.
>>>>> +     * @deprecated - one should be using the {@link
>>>>> PoissonSampler#sample()} method,
>>>>> +     *      * as it is considerably faster.
>>>>> +     */
>>>>> +    @Deprecated
>>>>> +    protected long nextLong() {
>>>>> +        return super.nextLong();
>>>>> +    }
>>>>> +
>>>>>    /** {@inheritDoc} */
>>>>>    @Override
>>>>>    public String toString() {
>>>>>



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

Rob Tompkins


> On Aug 9, 2018, at 8:38 AM, Gilles <[hidden email]> wrote:
>
> On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote:
>> Are we instead trying to remove the “extends” from all of those classes?
>
> I'm not sure what you mean, sorry.
>
> "SamplerBase" was meant has a utility/shortcut/boiler-plate in order
> to copy the "rng" argument in one place, instead of having a field
> in each sampler class. It is purely internal to this library (in C++,
> inheritance would have been "private”).

Are we trying to remove SamplerBase all together?

>
>> My thought was to override the method and javadoc locally (in the
>> actual subclass) so that we can give reasonable deprecation messages,
>> but that’s only a thought. I’m ok with whatever.
>
> The message would aim at the wrong target: for developers of this
> library, there is no deprecation (the boiler-plate code is there
> to be used); for application developers, the class should not be
> there to be used (hence: package private).

Then, I guess, we should simply state that we want to eventually delete these methods and they should not be consumed. Yeah? Like I said…I’m up for whatever. I’m just trying to balance yours and Gary’s points and find a good middle ground that everyone can live with.

-Rob

>
> Gilles
>
>> -Rob
>>
>>> On Aug 9, 2018, at 7:02 AM, Gilles <[hidden email]> wrote:
>>>
>>> Hi all.
>>>
>>> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
>>>> Hello Rob.
>>>>
>>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>>>>> only a +0 on this change. So, if you want to revert it before going up
>>>>> with 1.1, that’s fine.
>>>>
>>>> I don't understand this change after what I answered to Gary's strange
>>>> proposal.
>>>> The comment is *wrong*.  As I said previously, the base class provides
>>>> boiler-plate code; that is *not* deprecated.  The issue is that those
>>>> methods were not meant to be used further down the hierarchy (in user's
>>>> subclasses). [And now, furthermore, they are not used anymore in class
>>>> "PoissonSampler", following the change in RNG-50 (delegation to other
>>>> classes).]
>>>> The sampler classes should have been made "final"; but now, this change
>>>> would also be "breaking" (even though I doubt about legitimate use-cases
>>>> for inherithing from the sampler implementations).
>>>
>>> Assuming it's unlikely that application developers would have
>>> * called protected methods of "SamplerBase",[1]
>>> * create subclasses of "SamplerBase",[1]
>>> * create subclasses of the sampler classes,[1]
>>> I suggest to
>>> * make "SamplerBase" package private
>>> * make the sampler clases "final".
>>>
>>> The above are the *less* intrusive fixes, as they would only
>>> potentially impact application developers by making them aware
>>> that they have made incorrect usage of an *inadvertently*
>>> public API.  Correct usage will not be impacted even though
>>> the change is not BC.[2]
>>>
>>> Any objection to that fix?[3]
>>>
>>> Regards,
>>> Gilles
>>>
>>> [1] Rationale: (a) There is no reason to do that and
>>>              (b) "Commons RNG" isn't much used at this point:
>>>     http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
>>> [2] I recall that a similar situation (BC breaking in a minor
>>>   release) occurred in CM, at a time where the number of
>>>   potential users was much larger.
>>> [3] I'll add a prominent warning in the changelog to the effect
>>>   that people wanting to continue with incorrect usage of the
>>>   samplers should not upgrade. ;-)
>>>
>>>>
>>>> Please revert.
>>>>
>>>> Thanks,
>>>> Gilles
>>>>
>>>>>
>>>>> -Rob
>>>>>
>>>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>>>>
>>>>>> Repository: commons-rng
>>>>>> Updated Branches:
>>>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>>>>
>>>>>>
>>>>>> Adding PoissonSampler deprecations. Use the correct faster public methods
>>>>>>
>>>>>>
>>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>>>> Commit: http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>>>>
>>>>>> Branch: refs/heads/1.1
>>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>>>> Parents: 50b984b
>>>>>> Author: Rob Tompkins <[hidden email]>
>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>>>> Committer: Rob Tompkins <[hidden email]>
>>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>>>>
>>>>>> ----------------------------------------------------------------------
>>>>>> .../sampling/distribution/PoissonSampler.java   | 42 ++++++++++++++++++++
>>>>>> 1 file changed, 42 insertions(+)
>>>>>> ----------------------------------------------------------------------
>>>>>>
>>>>>>
>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>> ----------------------------------------------------------------------
>>>>>> diff --git a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>> index d0733ba..29d0e4e 100644
>>>>>> --- a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>> +++ b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>>>       return poissonSampler.sample();
>>>>>>   }
>>>>>>
>>>>>> +    /**
>>>>>> +     * @return a random value from a uniform distribution in the
>>>>>> +     * interval {@code [0, 1)}.
>>>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>>>> +     * as it is considerably faster.
>>>>>> +     */
>>>>>> +    @Deprecated
>>>>>> +    protected double nextDouble() {
>>>>>> +        return super.nextDouble();
>>>>>> +    }
>>>>>> +
>>>>>> +    /**
>>>>>> +     * @return a random {@code int} value.
>>>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>>>> +     * as it is considerably faster.
>>>>>> +     */
>>>>>> +    @Deprecated
>>>>>> +    protected int nextInt() {
>>>>>> +        return super.nextInt();
>>>>>> +    }
>>>>>> +
>>>>>> +    /**
>>>>>> +     * @param max Upper bound (excluded).
>>>>>> +     * @return a random {@code int} value in the interval {@code [0, max)}.
>>>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>>>> +     *      * as it is considerably faster.
>>>>>> +     */
>>>>>> +    @Deprecated
>>>>>> +    protected int nextInt(int max) {
>>>>>> +        return super.nextInt(max);
>>>>>> +    }
>>>>>> +
>>>>>> +    /**
>>>>>> +     * @return a random {@code long} value.
>>>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>>>> +     *      * as it is considerably faster.
>>>>>> +     */
>>>>>> +    @Deprecated
>>>>>> +    protected long nextLong() {
>>>>>> +        return super.nextLong();
>>>>>> +    }
>>>>>> +
>>>>>>   /** {@inheritDoc} */
>>>>>>   @Override
>>>>>>   public String toString() {
>>>>>>
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-rng git commit: Adding PoissonSampler deprecations. Use the correct faster public methods

Gilles Sadowski
In reply to this post by Rob Tompkins
On Thu, 9 Aug 2018 08:27:16 -0400, Rob Tompkins wrote:

>> On Aug 8, 2018, at 1:59 PM, Gilles <[hidden email]>
>> wrote:
>>
>> Hello Rob.
>>
>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>>> only a +0 on this change. So, if you want to revert it before going
>>> up
>>> with 1.1, that’s fine.
>>
>> I don't understand this change after what I answered to Gary's
>> strange
>> proposal.
>> The comment is *wrong*.  As I said previously, the base class
>> provides
>> boiler-plate code; that is *not* deprecated.  The issue is that
>> those
>> methods were not meant to be used further down the hierarchy (in
>> user's
>> subclasses). [And now, furthermore, they are not used anymore in
>> class
>> "PoissonSampler", following the change in RNG-50 (delegation to
>> other
>> classes).]
>> The sampler classes should have been made "final"; but now, this
>> change
>> would also be "breaking" (even though I doubt about legitimate
>> use-cases
>> for inherithing from the sampler implementations).
>
> My goal was to have the class look like is has the same functionality
> at 1.0, but instead accommodate for local javadoc changes
> to point out what the user should be doing. That said, it’s entirely
> possible that I wasn’t precise enough with the “@deprecated”
> messaging
> due to lack of familiarity with the issue at hand.
>
> It is however unclear to me how these changes would be breaking as
> the methods exposed on the class don’t change signature or
> accessibility quantifier, but I could have overlooked something.

?
I'm saying that the appropriate change (that is: make the sampler
classes "final") would be breaking BC (in the sense that Clirr will
report errors).  However, users of this library most probably
wouldn't use it so as to be impacted by the change (that is: their
code will work as with 1.0).

Gilles

> Regardless, it was just an idea, and I figured the easiest way to
> make a proposal was to push a commit up. Like I said I’m a +0 on it,
> and if
> we come up with a better mechanism, then I’m on board with that.
>
> Cheers,
> -Rob
>
>>
>> Please revert.
>>
>> Thanks,
>> Gilles
>>
>>>
>>> -Rob
>>>
>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>>
>>>> Repository: commons-rng
>>>> Updated Branches:
>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>>
>>>>
>>>> Adding PoissonSampler deprecations. Use the correct faster public
>>>> methods
>>>>
>>>>
>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>> Commit:
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>> Tree:
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>> Diff:
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>>
>>>> Branch: refs/heads/1.1
>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>> Parents: 50b984b
>>>> Author: Rob Tompkins <[hidden email]>
>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>> Committer: Rob Tompkins <[hidden email]>
>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>> .../sampling/distribution/PoissonSampler.java   | 42
>>>> ++++++++++++++++++++
>>>> 1 file changed, 42 insertions(+)
>>>>
>>>> ----------------------------------------------------------------------
>>>>
>>>>
>>>>
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>
>>>> ----------------------------------------------------------------------
>>>> diff --git
>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> index d0733ba..29d0e4e 100644
>>>> ---
>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> +++
>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>        return poissonSampler.sample();
>>>>    }
>>>>
>>>> +    /**
>>>> +     * @return a random value from a uniform distribution in the
>>>> +     * interval {@code [0, 1)}.
>>>> +     * @deprecated - one should be using the {@link
>>>> PoissonSampler#sample()} method,
>>>> +     * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected double nextDouble() {
>>>> +        return super.nextDouble();
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * @return a random {@code int} value.
>>>> +     * @deprecated - one should be using the {@link
>>>> PoissonSampler#sample()} method,
>>>> +     * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected int nextInt() {
>>>> +        return super.nextInt();
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * @param max Upper bound (excluded).
>>>> +     * @return a random {@code int} value in the interval {@code
>>>> [0, max)}.
>>>> +     * @deprecated - one should be using the {@link
>>>> PoissonSampler#sample()} method,
>>>> +     *      * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected int nextInt(int max) {
>>>> +        return super.nextInt(max);
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * @return a random {@code long} value.
>>>> +     * @deprecated - one should be using the {@link
>>>> PoissonSampler#sample()} method,
>>>> +     *      * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected long nextLong() {
>>>> +        return super.nextLong();
>>>> +    }
>>>> +
>>>>    /** {@inheritDoc} */
>>>>    @Override
>>>>    public String toString() {
>>>>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-rng git commit: Adding PoissonSampler deprecations. Use the correct faster public methods

Rob Tompkins


> On Aug 9, 2018, at 8:47 AM, Gilles <[hidden email]> wrote:
>
> On Thu, 9 Aug 2018 08:27:16 -0400, Rob Tompkins wrote:
>>> On Aug 8, 2018, at 1:59 PM, Gilles <[hidden email]> wrote:
>>>
>>> Hello Rob.
>>>
>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>>>> only a +0 on this change. So, if you want to revert it before going up
>>>> with 1.1, that’s fine.
>>>
>>> I don't understand this change after what I answered to Gary's strange
>>> proposal.
>>> The comment is *wrong*.  As I said previously, the base class provides
>>> boiler-plate code; that is *not* deprecated.  The issue is that those
>>> methods were not meant to be used further down the hierarchy (in user's
>>> subclasses). [And now, furthermore, they are not used anymore in class
>>> "PoissonSampler", following the change in RNG-50 (delegation to other
>>> classes).]
>>> The sampler classes should have been made "final"; but now, this change
>>> would also be "breaking" (even though I doubt about legitimate use-cases
>>> for inherithing from the sampler implementations).
>>
>> My goal was to have the class look like is has the same functionality
>> at 1.0, but instead accommodate for local javadoc changes
>> to point out what the user should be doing. That said, it’s entirely
>> possible that I wasn’t precise enough with the “@deprecated” messaging
>> due to lack of familiarity with the issue at hand.
>>
>> It is however unclear to me how these changes would be breaking as
>> the methods exposed on the class don’t change signature or
>> accessibility quantifier, but I could have overlooked something.
>
> ?
> I'm saying that the appropriate change (that is: make the sampler
> classes "final") would be breaking BC (in the sense that Clirr will
> report errors).  However, users of this library most probably
> wouldn't use it so as to be impacted by the change (that is: their
> code will work as with 1.0).

Right, but we’re stuck not being able to make the appropriate change for 1.1. So we have to find the next best thing, and I’m just poking around trying to find anything that works in that category.

>
> Gilles
>
>> Regardless, it was just an idea, and I figured the easiest way to
>> make a proposal was to push a commit up. Like I said I’m a +0 on it,
>> and if
>> we come up with a better mechanism, then I’m on board with that.
>>
>> Cheers,
>> -Rob
>>
>>>
>>> Please revert.
>>>
>>> Thanks,
>>> Gilles
>>>
>>>>
>>>> -Rob
>>>>
>>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>>>
>>>>> Repository: commons-rng
>>>>> Updated Branches:
>>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>>>
>>>>>
>>>>> Adding PoissonSampler deprecations. Use the correct faster public methods
>>>>>
>>>>>
>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>>> Commit: http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>>>
>>>>> Branch: refs/heads/1.1
>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>>> Parents: 50b984b
>>>>> Author: Rob Tompkins <[hidden email]>
>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>>> Committer: Rob Tompkins <[hidden email]>
>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>>>
>>>>> ----------------------------------------------------------------------
>>>>> .../sampling/distribution/PoissonSampler.java   | 42 ++++++++++++++++++++
>>>>> 1 file changed, 42 insertions(+)
>>>>> ----------------------------------------------------------------------
>>>>>
>>>>>
>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>> ----------------------------------------------------------------------
>>>>> diff --git a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>> index d0733ba..29d0e4e 100644
>>>>> --- a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>> +++ b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>>       return poissonSampler.sample();
>>>>>   }
>>>>>
>>>>> +    /**
>>>>> +     * @return a random value from a uniform distribution in the
>>>>> +     * interval {@code [0, 1)}.
>>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>>> +     * as it is considerably faster.
>>>>> +     */
>>>>> +    @Deprecated
>>>>> +    protected double nextDouble() {
>>>>> +        return super.nextDouble();
>>>>> +    }
>>>>> +
>>>>> +    /**
>>>>> +     * @return a random {@code int} value.
>>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>>> +     * as it is considerably faster.
>>>>> +     */
>>>>> +    @Deprecated
>>>>> +    protected int nextInt() {
>>>>> +        return super.nextInt();
>>>>> +    }
>>>>> +
>>>>> +    /**
>>>>> +     * @param max Upper bound (excluded).
>>>>> +     * @return a random {@code int} value in the interval {@code [0, max)}.
>>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>>> +     *      * as it is considerably faster.
>>>>> +     */
>>>>> +    @Deprecated
>>>>> +    protected int nextInt(int max) {
>>>>> +        return super.nextInt(max);
>>>>> +    }
>>>>> +
>>>>> +    /**
>>>>> +     * @return a random {@code long} value.
>>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>>> +     *      * as it is considerably faster.
>>>>> +     */
>>>>> +    @Deprecated
>>>>> +    protected long nextLong() {
>>>>> +        return super.nextLong();
>>>>> +    }
>>>>> +
>>>>>   /** {@inheritDoc} */
>>>>>   @Override
>>>>>   public String toString() {
>>>>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

Gilles Sadowski
In reply to this post by Rob Tompkins
On Thu, 9 Aug 2018 08:43:34 -0400, Rob Tompkins wrote:

>> On Aug 9, 2018, at 8:38 AM, Gilles <[hidden email]>
>> wrote:
>>
>> On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote:
>>> Are we instead trying to remove the “extends” from all of those
>>> classes?
>>
>> I'm not sure what you mean, sorry.
>>
>> "SamplerBase" was meant has a utility/shortcut/boiler-plate in order
>> to copy the "rng" argument in one place, instead of having a field
>> in each sampler class. It is purely internal to this library (in
>> C++,
>> inheritance would have been "private”).
>
> Are we trying to remove SamplerBase all together?

That would break BC the same way.
I wouldn't mind since the shortcut is not really significant.

But my reasoning would be the same: correct usage does not refer
to these methods so the fix could be done now, with minimal risk
(due to low usage of a new component).

>>> My thought was to override the method and javadoc locally (in the
>>> actual subclass) so that we can give reasonable deprecation
>>> messages,
>>> but that’s only a thought. I’m ok with whatever.

It's overkill in this context.
We shouldn't be rigid about the "no BC in minor release" rule;
enforcing it is worse for everybody:
  * bona fide users will be forced to modify their source and
    recompile in order to benefit from the performance boost
    introduced in version 1.1, and
  * bad usage (if it exists at all) could continue unsuspected.

>> The message would aim at the wrong target: for developers of this
>> library, there is no deprecation (the boiler-plate code is there
>> to be used); for application developers, the class should not be
>> there to be used (hence: package private).
>
> Then, I guess, we should simply state that we want to eventually
> delete these methods and they should not be consumed. Yeah?

No (cf. above); I propose to delete them *now*.

> Like I
> said…I’m up for whatever. I’m just trying to balance yours and Gary’s
> points and find a good middle ground that everyone can live with.

Which are the two sides for which you try to find a middle ground?
At any rate it would not be "good" if it makes developers and users
unhappy for a case that probably doesn't exist out there.
Tools such as Clirr are a great help, but they shouldn't induce us
to take the wrong course only to have the pleasure to contemplate a
clean report. ;-)
Thanks to Clirr (and Gary), we've become aware of a design issue.
We can decide to resolve it as it was done in the past, with IMHO
zero inconvenience (except for the Clirr "unclean" report).

Regards,
Gilles

> -Rob
>
>>
>> Gilles
>>
>>> -Rob
>>>
>>>> On Aug 9, 2018, at 7:02 AM, Gilles <[hidden email]>
>>>> wrote:
>>>>
>>>> Hi all.
>>>>
>>>> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
>>>>> Hello Rob.
>>>>>
>>>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>>>>> @Gilles - thoughts here?? Just kinda what I was thinking, but
>>>>>> I’m
>>>>>> only a +0 on this change. So, if you want to revert it before
>>>>>> going up
>>>>>> with 1.1, that’s fine.
>>>>>
>>>>> I don't understand this change after what I answered to Gary's
>>>>> strange
>>>>> proposal.
>>>>> The comment is *wrong*.  As I said previously, the base class
>>>>> provides
>>>>> boiler-plate code; that is *not* deprecated.  The issue is that
>>>>> those
>>>>> methods were not meant to be used further down the hierarchy (in
>>>>> user's
>>>>> subclasses). [And now, furthermore, they are not used anymore in
>>>>> class
>>>>> "PoissonSampler", following the change in RNG-50 (delegation to
>>>>> other
>>>>> classes).]
>>>>> The sampler classes should have been made "final"; but now, this
>>>>> change
>>>>> would also be "breaking" (even though I doubt about legitimate
>>>>> use-cases
>>>>> for inherithing from the sampler implementations).
>>>>
>>>> Assuming it's unlikely that application developers would have
>>>> * called protected methods of "SamplerBase",[1]
>>>> * create subclasses of "SamplerBase",[1]
>>>> * create subclasses of the sampler classes,[1]
>>>> I suggest to
>>>> * make "SamplerBase" package private
>>>> * make the sampler clases "final".
>>>>
>>>> The above are the *less* intrusive fixes, as they would only
>>>> potentially impact application developers by making them aware
>>>> that they have made incorrect usage of an *inadvertently*
>>>> public API.  Correct usage will not be impacted even though
>>>> the change is not BC.[2]
>>>>
>>>> Any objection to that fix?[3]
>>>>
>>>> Regards,
>>>> Gilles
>>>>
>>>> [1] Rationale: (a) There is no reason to do that and
>>>>              (b) "Commons RNG" isn't much used at this point:
>>>>    
>>>> http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
>>>> [2] I recall that a similar situation (BC breaking in a minor
>>>>   release) occurred in CM, at a time where the number of
>>>>   potential users was much larger.
>>>> [3] I'll add a prominent warning in the changelog to the effect
>>>>   that people wanting to continue with incorrect usage of the
>>>>   samplers should not upgrade. ;-)
>>>>
>>>>>
>>>>> Please revert.
>>>>>
>>>>> Thanks,
>>>>> Gilles
>>>>>
>>>>>>
>>>>>> -Rob
>>>>>>
>>>>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>>>>>
>>>>>>> Repository: commons-rng
>>>>>>> Updated Branches:
>>>>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>>>>>
>>>>>>>
>>>>>>> Adding PoissonSampler deprecations. Use the correct faster
>>>>>>> public methods
>>>>>>>
>>>>>>>
>>>>>>> Project:
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>>>>> Commit:
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>>>>> Tree:
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>>>>> Diff:
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>>>>>
>>>>>>> Branch: refs/heads/1.1
>>>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>>>>> Parents: 50b984b
>>>>>>> Author: Rob Tompkins <[hidden email]>
>>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>>>>> Committer: Rob Tompkins <[hidden email]>
>>>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>>>>>
>>>>>>>
>>>>>>> ----------------------------------------------------------------------
>>>>>>> .../sampling/distribution/PoissonSampler.java   | 42
>>>>>>> ++++++++++++++++++++
>>>>>>> 1 file changed, 42 insertions(+)
>>>>>>>
>>>>>>> ----------------------------------------------------------------------
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>
>>>>>>> ----------------------------------------------------------------------
>>>>>>> diff --git
>>>>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>> index d0733ba..29d0e4e 100644
>>>>>>> ---
>>>>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>> +++
>>>>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>>>>       return poissonSampler.sample();
>>>>>>>   }
>>>>>>>
>>>>>>> +    /**
>>>>>>> +     * @return a random value from a uniform distribution in
>>>>>>> the
>>>>>>> +     * interval {@code [0, 1)}.
>>>>>>> +     * @deprecated - one should be using the {@link
>>>>>>> PoissonSampler#sample()} method,
>>>>>>> +     * as it is considerably faster.
>>>>>>> +     */
>>>>>>> +    @Deprecated
>>>>>>> +    protected double nextDouble() {
>>>>>>> +        return super.nextDouble();
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    /**
>>>>>>> +     * @return a random {@code int} value.
>>>>>>> +     * @deprecated - one should be using the {@link
>>>>>>> PoissonSampler#sample()} method,
>>>>>>> +     * as it is considerably faster.
>>>>>>> +     */
>>>>>>> +    @Deprecated
>>>>>>> +    protected int nextInt() {
>>>>>>> +        return super.nextInt();
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    /**
>>>>>>> +     * @param max Upper bound (excluded).
>>>>>>> +     * @return a random {@code int} value in the interval
>>>>>>> {@code [0, max)}.
>>>>>>> +     * @deprecated - one should be using the {@link
>>>>>>> PoissonSampler#sample()} method,
>>>>>>> +     *      * as it is considerably faster.
>>>>>>> +     */
>>>>>>> +    @Deprecated
>>>>>>> +    protected int nextInt(int max) {
>>>>>>> +        return super.nextInt(max);
>>>>>>> +    }
>>>>>>> +
>>>>>>> +    /**
>>>>>>> +     * @return a random {@code long} value.
>>>>>>> +     * @deprecated - one should be using the {@link
>>>>>>> PoissonSampler#sample()} method,
>>>>>>> +     *      * as it is considerably faster.
>>>>>>> +     */
>>>>>>> +    @Deprecated
>>>>>>> +    protected long nextLong() {
>>>>>>> +        return super.nextLong();
>>>>>>> +    }
>>>>>>> +
>>>>>>>   /** {@inheritDoc} */
>>>>>>>   @Override
>>>>>>>   public String toString() {
>>>>>>>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: commons-rng git commit: Adding PoissonSampler deprecations. Use the correct faster public methods

Gilles Sadowski
In reply to this post by Rob Tompkins
On Thu, 9 Aug 2018 08:50:11 -0400, Rob Tompkins wrote:

>> On Aug 9, 2018, at 8:47 AM, Gilles <[hidden email]>
>> wrote:
>>
>> On Thu, 9 Aug 2018 08:27:16 -0400, Rob Tompkins wrote:
>>>> On Aug 8, 2018, at 1:59 PM, Gilles <[hidden email]>
>>>> wrote:
>>>>
>>>> Hello Rob.
>>>>
>>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>>>>> only a +0 on this change. So, if you want to revert it before
>>>>> going up
>>>>> with 1.1, that’s fine.
>>>>
>>>> I don't understand this change after what I answered to Gary's
>>>> strange
>>>> proposal.
>>>> The comment is *wrong*.  As I said previously, the base class
>>>> provides
>>>> boiler-plate code; that is *not* deprecated.  The issue is that
>>>> those
>>>> methods were not meant to be used further down the hierarchy (in
>>>> user's
>>>> subclasses). [And now, furthermore, they are not used anymore in
>>>> class
>>>> "PoissonSampler", following the change in RNG-50 (delegation to
>>>> other
>>>> classes).]
>>>> The sampler classes should have been made "final"; but now, this
>>>> change
>>>> would also be "breaking" (even though I doubt about legitimate
>>>> use-cases
>>>> for inherithing from the sampler implementations).
>>>
>>> My goal was to have the class look like is has the same
>>> functionality
>>> at 1.0, but instead accommodate for local javadoc changes
>>> to point out what the user should be doing. That said, it’s
>>> entirely
>>> possible that I wasn’t precise enough with the “@deprecated”
>>> messaging
>>> due to lack of familiarity with the issue at hand.
>>>
>>> It is however unclear to me how these changes would be breaking as
>>> the methods exposed on the class don’t change signature or
>>> accessibility quantifier, but I could have overlooked something.
>>
>> ?
>> I'm saying that the appropriate change (that is: make the sampler
>> classes "final") would be breaking BC (in the sense that Clirr will
>> report errors).  However, users of this library most probably
>> wouldn't use it so as to be impacted by the change (that is: their
>> code will work as with 1.0).
>
> Right, but we’re stuck not being able to make the appropriate change
> for 1.1. So we have to find the next best thing, and I’m just poking
> around trying to find anything that works in that category.

We would be stuck only if the goal is to produce a clean report
rather than to provide a good library.  Cf. other post sent a
few moments ago.

Regards,
Gilles

>>
>>> Regardless, it was just an idea, and I figured the easiest way to
>>> make a proposal was to push a commit up. Like I said I’m a +0 on
>>> it,
>>> and if
>>> we come up with a better mechanism, then I’m on board with that.
>>>
>>> Cheers,
>>> -Rob
>>>
>>>>
>>>> Please revert.
>>>>
>>>> Thanks,
>>>> Gilles
>>>>
>>>>>
>>>>> -Rob
>>>>>
>>>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>>>>
>>>>>> Repository: commons-rng
>>>>>> Updated Branches:
>>>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>>>>
>>>>>>
>>>>>> Adding PoissonSampler deprecations. Use the correct faster
>>>>>> public methods
>>>>>>
>>>>>>
>>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>>>> Commit:
>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>>>> Tree:
>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>>>> Diff:
>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>>>>
>>>>>> Branch: refs/heads/1.1
>>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>>>> Parents: 50b984b
>>>>>> Author: Rob Tompkins <[hidden email]>
>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>>>> Committer: Rob Tompkins <[hidden email]>
>>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>>>>
>>>>>>
>>>>>> ----------------------------------------------------------------------
>>>>>> .../sampling/distribution/PoissonSampler.java   | 42
>>>>>> ++++++++++++++++++++
>>>>>> 1 file changed, 42 insertions(+)
>>>>>>
>>>>>> ----------------------------------------------------------------------
>>>>>>
>>>>>>
>>>>>>
>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>
>>>>>> ----------------------------------------------------------------------
>>>>>> diff --git
>>>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>> index d0733ba..29d0e4e 100644
>>>>>> ---
>>>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>> +++
>>>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>>>       return poissonSampler.sample();
>>>>>>   }
>>>>>>
>>>>>> +    /**
>>>>>> +     * @return a random value from a uniform distribution in
>>>>>> the
>>>>>> +     * interval {@code [0, 1)}.
>>>>>> +     * @deprecated - one should be using the {@link
>>>>>> PoissonSampler#sample()} method,
>>>>>> +     * as it is considerably faster.
>>>>>> +     */
>>>>>> +    @Deprecated
>>>>>> +    protected double nextDouble() {
>>>>>> +        return super.nextDouble();
>>>>>> +    }
>>>>>> +
>>>>>> +    /**
>>>>>> +     * @return a random {@code int} value.
>>>>>> +     * @deprecated - one should be using the {@link
>>>>>> PoissonSampler#sample()} method,
>>>>>> +     * as it is considerably faster.
>>>>>> +     */
>>>>>> +    @Deprecated
>>>>>> +    protected int nextInt() {
>>>>>> +        return super.nextInt();
>>>>>> +    }
>>>>>> +
>>>>>> +    /**
>>>>>> +     * @param max Upper bound (excluded).
>>>>>> +     * @return a random {@code int} value in the interval
>>>>>> {@code [0, max)}.
>>>>>> +     * @deprecated - one should be using the {@link
>>>>>> PoissonSampler#sample()} method,
>>>>>> +     *      * as it is considerably faster.
>>>>>> +     */
>>>>>> +    @Deprecated
>>>>>> +    protected int nextInt(int max) {
>>>>>> +        return super.nextInt(max);
>>>>>> +    }
>>>>>> +
>>>>>> +    /**
>>>>>> +     * @return a random {@code long} value.
>>>>>> +     * @deprecated - one should be using the {@link
>>>>>> PoissonSampler#sample()} method,
>>>>>> +     *      * as it is considerably faster.
>>>>>> +     */
>>>>>> +    @Deprecated
>>>>>> +    protected long nextLong() {
>>>>>> +        return super.nextLong();
>>>>>> +    }
>>>>>> +
>>>>>>   /** {@inheritDoc} */
>>>>>>   @Override
>>>>>>   public String toString() {
>>>>>>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

Rob Tompkins
In reply to this post by Gilles Sadowski


> On Aug 9, 2018, at 9:17 AM, Gilles <[hidden email]> wrote:
>
> On Thu, 9 Aug 2018 08:43:34 -0400, Rob Tompkins wrote:
>>> On Aug 9, 2018, at 8:38 AM, Gilles <[hidden email]> wrote:
>>>
>>> On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote:
>>>> Are we instead trying to remove the “extends” from all of those classes?
>>>
>>> I'm not sure what you mean, sorry.
>>>
>>> "SamplerBase" was meant has a utility/shortcut/boiler-plate in order
>>> to copy the "rng" argument in one place, instead of having a field
>>> in each sampler class. It is purely internal to this library (in C++,
>>> inheritance would have been "private”).
>>
>> Are we trying to remove SamplerBase all together?
>
> That would break BC the same way.
> I wouldn't mind since the shortcut is not really significant.
>
> But my reasoning would be the same: correct usage does not refer
> to these methods so the fix could be done now, with minimal risk
> (due to low usage of a new component).
>
>>>> My thought was to override the method and javadoc locally (in the
>>>> actual subclass) so that we can give reasonable deprecation messages,
>>>> but that’s only a thought. I’m ok with whatever.
>
> It's overkill in this context.
> We shouldn't be rigid about the "no BC in minor release" rule;

I’m a tad confused here. I thought that the whole point of everything that we do in terms of the way we version and name packages
was specifically driven at the fact that we’re completely rigid about “no BC breakage in minor releases.”

I’ve not been around the project long enough to know this with certainty, but that definitely is the vibe that I get. I would think that such RCs would generally get -1’s from people.

Generally I’m open to whatever. I’m just operating on what I perceive to be precedent.

@Gary - what’s the call here on the flexibility of BC incompatible changes in a minor release?

> enforcing it is worse for everybody:
> * bona fide users will be forced to modify their source and
>   recompile in order to benefit from the performance boost
>   introduced in version 1.1, and
> * bad usage (if it exists at all) could continue unsuspected.
>
>>> The message would aim at the wrong target: for developers of this
>>> library, there is no deprecation (the boiler-plate code is there
>>> to be used); for application developers, the class should not be
>>> there to be used (hence: package private).
>>
>> Then, I guess, we should simply state that we want to eventually
>> delete these methods and they should not be consumed. Yeah?
>
> No (cf. above); I propose to delete them *now*.
>
>> Like I
>> said…I’m up for whatever. I’m just trying to balance yours and Gary’s
>> points and find a good middle ground that everyone can live with.
>
> Which are the two sides for which you try to find a middle ground?
> At any rate it would not be "good" if it makes developers and users
> unhappy for a case that probably doesn't exist out there.
> Tools such as Clirr are a great help, but they shouldn't induce us
> to take the wrong course only to have the pleasure to contemplate a
> clean report. ;-)
> Thanks to Clirr (and Gary), we've become aware of a design issue.
> We can decide to resolve it as it was done in the past, with IMHO
> zero inconvenience (except for the Clirr "unclean" report).
>
> Regards,
> Gilles
>
>> -Rob
>>
>>>
>>> Gilles
>>>
>>>> -Rob
>>>>
>>>>> On Aug 9, 2018, at 7:02 AM, Gilles <[hidden email]> wrote:
>>>>>
>>>>> Hi all.
>>>>>
>>>>> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
>>>>>> Hello Rob.
>>>>>>
>>>>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>>>>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>>>>>>> only a +0 on this change. So, if you want to revert it before going up
>>>>>>> with 1.1, that’s fine.
>>>>>>
>>>>>> I don't understand this change after what I answered to Gary's strange
>>>>>> proposal.
>>>>>> The comment is *wrong*.  As I said previously, the base class provides
>>>>>> boiler-plate code; that is *not* deprecated.  The issue is that those
>>>>>> methods were not meant to be used further down the hierarchy (in user's
>>>>>> subclasses). [And now, furthermore, they are not used anymore in class
>>>>>> "PoissonSampler", following the change in RNG-50 (delegation to other
>>>>>> classes).]
>>>>>> The sampler classes should have been made "final"; but now, this change
>>>>>> would also be "breaking" (even though I doubt about legitimate use-cases
>>>>>> for inherithing from the sampler implementations).
>>>>>
>>>>> Assuming it's unlikely that application developers would have
>>>>> * called protected methods of "SamplerBase",[1]
>>>>> * create subclasses of "SamplerBase",[1]
>>>>> * create subclasses of the sampler classes,[1]
>>>>> I suggest to
>>>>> * make "SamplerBase" package private
>>>>> * make the sampler clases "final".
>>>>>
>>>>> The above are the *less* intrusive fixes, as they would only
>>>>> potentially impact application developers by making them aware
>>>>> that they have made incorrect usage of an *inadvertently*
>>>>> public API.  Correct usage will not be impacted even though
>>>>> the change is not BC.[2]
>>>>>
>>>>> Any objection to that fix?[3]
>>>>>
>>>>> Regards,
>>>>> Gilles
>>>>>
>>>>> [1] Rationale: (a) There is no reason to do that and
>>>>>             (b) "Commons RNG" isn't much used at this point:
>>>>>    http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
>>>>> [2] I recall that a similar situation (BC breaking in a minor
>>>>>  release) occurred in CM, at a time where the number of
>>>>>  potential users was much larger.
>>>>> [3] I'll add a prominent warning in the changelog to the effect
>>>>>  that people wanting to continue with incorrect usage of the
>>>>>  samplers should not upgrade. ;-)
>>>>>
>>>>>>
>>>>>> Please revert.
>>>>>>
>>>>>> Thanks,
>>>>>> Gilles
>>>>>>
>>>>>>>
>>>>>>> -Rob
>>>>>>>
>>>>>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>>>>>>
>>>>>>>> Repository: commons-rng
>>>>>>>> Updated Branches:
>>>>>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>>>>>>
>>>>>>>>
>>>>>>>> Adding PoissonSampler deprecations. Use the correct faster public methods
>>>>>>>>
>>>>>>>>
>>>>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>>>>>> Commit: http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>>>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>>>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>>>>>>
>>>>>>>> Branch: refs/heads/1.1
>>>>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>>>>>> Parents: 50b984b
>>>>>>>> Author: Rob Tompkins <[hidden email]>
>>>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>>>>>> Committer: Rob Tompkins <[hidden email]>
>>>>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>>>>>>
>>>>>>>> ----------------------------------------------------------------------
>>>>>>>> .../sampling/distribution/PoissonSampler.java   | 42 ++++++++++++++++++++
>>>>>>>> 1 file changed, 42 insertions(+)
>>>>>>>> ----------------------------------------------------------------------
>>>>>>>>
>>>>>>>>
>>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>> ----------------------------------------------------------------------
>>>>>>>> diff --git a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>> index d0733ba..29d0e4e 100644
>>>>>>>> --- a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>> +++ b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>>>>>      return poissonSampler.sample();
>>>>>>>>  }
>>>>>>>>
>>>>>>>> +    /**
>>>>>>>> +     * @return a random value from a uniform distribution in the
>>>>>>>> +     * interval {@code [0, 1)}.
>>>>>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>>>>>> +     * as it is considerably faster.
>>>>>>>> +     */
>>>>>>>> +    @Deprecated
>>>>>>>> +    protected double nextDouble() {
>>>>>>>> +        return super.nextDouble();
>>>>>>>> +    }
>>>>>>>> +
>>>>>>>> +    /**
>>>>>>>> +     * @return a random {@code int} value.
>>>>>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>>>>>> +     * as it is considerably faster.
>>>>>>>> +     */
>>>>>>>> +    @Deprecated
>>>>>>>> +    protected int nextInt() {
>>>>>>>> +        return super.nextInt();
>>>>>>>> +    }
>>>>>>>> +
>>>>>>>> +    /**
>>>>>>>> +     * @param max Upper bound (excluded).
>>>>>>>> +     * @return a random {@code int} value in the interval {@code [0, max)}.
>>>>>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>>>>>> +     *      * as it is considerably faster.
>>>>>>>> +     */
>>>>>>>> +    @Deprecated
>>>>>>>> +    protected int nextInt(int max) {
>>>>>>>> +        return super.nextInt(max);
>>>>>>>> +    }
>>>>>>>> +
>>>>>>>> +    /**
>>>>>>>> +     * @return a random {@code long} value.
>>>>>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>>>>>> +     *      * as it is considerably faster.
>>>>>>>> +     */
>>>>>>>> +    @Deprecated
>>>>>>>> +    protected long nextLong() {
>>>>>>>> +        return super.nextLong();
>>>>>>>> +    }
>>>>>>>> +
>>>>>>>>  /** {@inheritDoc} */
>>>>>>>>  @Override
>>>>>>>>  public String toString() {
>>>>>>>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

Gilles Sadowski
On Thu, 9 Aug 2018 09:25:13 -0400, Rob Tompkins wrote:

>> On Aug 9, 2018, at 9:17 AM, Gilles <[hidden email]>
>> wrote:
>>
>> On Thu, 9 Aug 2018 08:43:34 -0400, Rob Tompkins wrote:
>>>> On Aug 9, 2018, at 8:38 AM, Gilles <[hidden email]>
>>>> wrote:
>>>>
>>>> On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote:
>>>>> Are we instead trying to remove the “extends” from all of those
>>>>> classes?
>>>>
>>>> I'm not sure what you mean, sorry.
>>>>
>>>> "SamplerBase" was meant has a utility/shortcut/boiler-plate in
>>>> order
>>>> to copy the "rng" argument in one place, instead of having a field
>>>> in each sampler class. It is purely internal to this library (in
>>>> C++,
>>>> inheritance would have been "private”).
>>>
>>> Are we trying to remove SamplerBase all together?
>>
>> That would break BC the same way.
>> I wouldn't mind since the shortcut is not really significant.
>>
>> But my reasoning would be the same: correct usage does not refer
>> to these methods so the fix could be done now, with minimal risk
>> (due to low usage of a new component).
>>
>>>>> My thought was to override the method and javadoc locally (in the
>>>>> actual subclass) so that we can give reasonable deprecation
>>>>> messages,
>>>>> but that’s only a thought. I’m ok with whatever.
>>
>> It's overkill in this context.
>> We shouldn't be rigid about the "no BC in minor release" rule;
>
> I’m a tad confused here. I thought that the whole point of everything
> that we do in terms of the way we version and name packages
> was specifically driven at the fact that we’re completely rigid about
> “no BC breakage in minor releases.”
>
> I’ve not been around the project long enough to know this with
> certainty, but that definitely is the vibe that I get. I would think
> that such RCs would generally get -1’s from people.

Right. Except when the rationale for breaking BC shows that no bona
fide codes can suffer from the incompatibility.  This had already
been discussed, and accepted, for this very release a few months
ago (cf. "BoxMullerLogNormalSampler"[1]).

> Generally I’m open to whatever. I’m just operating on what I perceive
> to be precedent.

I understand but there is a slight difference between being nice
to people and getting clean reports; don't you think? ;-)

Gilles

[1] https://issues.apache.org/jira/browse/RNG-46

> @Gary - what’s the call here on the flexibility of BC incompatible
> changes in a minor release?
>
>> enforcing it is worse for everybody:
>> * bona fide users will be forced to modify their source and
>>   recompile in order to benefit from the performance boost
>>   introduced in version 1.1, and
>> * bad usage (if it exists at all) could continue unsuspected.
>>
>>>> The message would aim at the wrong target: for developers of this
>>>> library, there is no deprecation (the boiler-plate code is there
>>>> to be used); for application developers, the class should not be
>>>> there to be used (hence: package private).
>>>
>>> Then, I guess, we should simply state that we want to eventually
>>> delete these methods and they should not be consumed. Yeah?
>>
>> No (cf. above); I propose to delete them *now*.
>>
>>> Like I
>>> said…I’m up for whatever. I’m just trying to balance yours and
>>> Gary’s
>>> points and find a good middle ground that everyone can live with.
>>
>> Which are the two sides for which you try to find a middle ground?
>> At any rate it would not be "good" if it makes developers and users
>> unhappy for a case that probably doesn't exist out there.
>> Tools such as Clirr are a great help, but they shouldn't induce us
>> to take the wrong course only to have the pleasure to contemplate a
>> clean report. ;-)
>> Thanks to Clirr (and Gary), we've become aware of a design issue.
>> We can decide to resolve it as it was done in the past, with IMHO
>> zero inconvenience (except for the Clirr "unclean" report).
>>
>> Regards,
>> Gilles
>>
>>> -Rob
>>>
>>>>
>>>> Gilles
>>>>
>>>>> -Rob
>>>>>
>>>>>> On Aug 9, 2018, at 7:02 AM, Gilles
>>>>>> <[hidden email]> wrote:
>>>>>>
>>>>>> Hi all.
>>>>>>
>>>>>> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
>>>>>>> Hello Rob.
>>>>>>>
>>>>>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>>>>>>> @Gilles - thoughts here?? Just kinda what I was thinking, but
>>>>>>>> I’m
>>>>>>>> only a +0 on this change. So, if you want to revert it before
>>>>>>>> going up
>>>>>>>> with 1.1, that’s fine.
>>>>>>>
>>>>>>> I don't understand this change after what I answered to Gary's
>>>>>>> strange
>>>>>>> proposal.
>>>>>>> The comment is *wrong*.  As I said previously, the base class
>>>>>>> provides
>>>>>>> boiler-plate code; that is *not* deprecated.  The issue is that
>>>>>>> those
>>>>>>> methods were not meant to be used further down the hierarchy
>>>>>>> (in user's
>>>>>>> subclasses). [And now, furthermore, they are not used anymore
>>>>>>> in class
>>>>>>> "PoissonSampler", following the change in RNG-50 (delegation to
>>>>>>> other
>>>>>>> classes).]
>>>>>>> The sampler classes should have been made "final"; but now,
>>>>>>> this change
>>>>>>> would also be "breaking" (even though I doubt about legitimate
>>>>>>> use-cases
>>>>>>> for inherithing from the sampler implementations).
>>>>>>
>>>>>> Assuming it's unlikely that application developers would have
>>>>>> * called protected methods of "SamplerBase",[1]
>>>>>> * create subclasses of "SamplerBase",[1]
>>>>>> * create subclasses of the sampler classes,[1]
>>>>>> I suggest to
>>>>>> * make "SamplerBase" package private
>>>>>> * make the sampler clases "final".
>>>>>>
>>>>>> The above are the *less* intrusive fixes, as they would only
>>>>>> potentially impact application developers by making them aware
>>>>>> that they have made incorrect usage of an *inadvertently*
>>>>>> public API.  Correct usage will not be impacted even though
>>>>>> the change is not BC.[2]
>>>>>>
>>>>>> Any objection to that fix?[3]
>>>>>>
>>>>>> Regards,
>>>>>> Gilles
>>>>>>
>>>>>> [1] Rationale: (a) There is no reason to do that and
>>>>>>             (b) "Commons RNG" isn't much used at this point:
>>>>>>    
>>>>>> http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
>>>>>> [2] I recall that a similar situation (BC breaking in a minor
>>>>>>  release) occurred in CM, at a time where the number of
>>>>>>  potential users was much larger.
>>>>>> [3] I'll add a prominent warning in the changelog to the effect
>>>>>>  that people wanting to continue with incorrect usage of the
>>>>>>  samplers should not upgrade. ;-)
>>>>>>
>>>>>>>
>>>>>>> Please revert.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Gilles
>>>>>>>
>>>>>>>>
>>>>>>>> -Rob
>>>>>>>>
>>>>>>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>>>>>>>
>>>>>>>>> Repository: commons-rng
>>>>>>>>> Updated Branches:
>>>>>>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Adding PoissonSampler deprecations. Use the correct faster
>>>>>>>>> public methods
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Project:
>>>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>>>>>>> Commit:
>>>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>>>>>>> Tree:
>>>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>>>>>>> Diff:
>>>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>>>>>>>
>>>>>>>>> Branch: refs/heads/1.1
>>>>>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>>>>>>> Parents: 50b984b
>>>>>>>>> Author: Rob Tompkins <[hidden email]>
>>>>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>>>>>>> Committer: Rob Tompkins <[hidden email]>
>>>>>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ----------------------------------------------------------------------
>>>>>>>>> .../sampling/distribution/PoissonSampler.java   | 42
>>>>>>>>> ++++++++++++++++++++
>>>>>>>>> 1 file changed, 42 insertions(+)
>>>>>>>>>
>>>>>>>>> ----------------------------------------------------------------------
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>>
>>>>>>>>> ----------------------------------------------------------------------
>>>>>>>>> diff --git
>>>>>>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>> index d0733ba..29d0e4e 100644
>>>>>>>>> ---
>>>>>>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>> +++
>>>>>>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>>>>>>      return poissonSampler.sample();
>>>>>>>>>  }
>>>>>>>>>
>>>>>>>>> +    /**
>>>>>>>>> +     * @return a random value from a uniform distribution in
>>>>>>>>> the
>>>>>>>>> +     * interval {@code [0, 1)}.
>>>>>>>>> +     * @deprecated - one should be using the {@link
>>>>>>>>> PoissonSampler#sample()} method,
>>>>>>>>> +     * as it is considerably faster.
>>>>>>>>> +     */
>>>>>>>>> +    @Deprecated
>>>>>>>>> +    protected double nextDouble() {
>>>>>>>>> +        return super.nextDouble();
>>>>>>>>> +    }
>>>>>>>>> +
>>>>>>>>> +    /**
>>>>>>>>> +     * @return a random {@code int} value.
>>>>>>>>> +     * @deprecated - one should be using the {@link
>>>>>>>>> PoissonSampler#sample()} method,
>>>>>>>>> +     * as it is considerably faster.
>>>>>>>>> +     */
>>>>>>>>> +    @Deprecated
>>>>>>>>> +    protected int nextInt() {
>>>>>>>>> +        return super.nextInt();
>>>>>>>>> +    }
>>>>>>>>> +
>>>>>>>>> +    /**
>>>>>>>>> +     * @param max Upper bound (excluded).
>>>>>>>>> +     * @return a random {@code int} value in the interval
>>>>>>>>> {@code [0, max)}.
>>>>>>>>> +     * @deprecated - one should be using the {@link
>>>>>>>>> PoissonSampler#sample()} method,
>>>>>>>>> +     *      * as it is considerably faster.
>>>>>>>>> +     */
>>>>>>>>> +    @Deprecated
>>>>>>>>> +    protected int nextInt(int max) {
>>>>>>>>> +        return super.nextInt(max);
>>>>>>>>> +    }
>>>>>>>>> +
>>>>>>>>> +    /**
>>>>>>>>> +     * @return a random {@code long} value.
>>>>>>>>> +     * @deprecated - one should be using the {@link
>>>>>>>>> PoissonSampler#sample()} method,
>>>>>>>>> +     *      * as it is considerably faster.
>>>>>>>>> +     */
>>>>>>>>> +    @Deprecated
>>>>>>>>> +    protected long nextLong() {
>>>>>>>>> +        return super.nextLong();
>>>>>>>>> +    }
>>>>>>>>> +
>>>>>>>>>  /** {@inheritDoc} */
>>>>>>>>>  @Override
>>>>>>>>>  public String toString() {
>>>>>>>>>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

garydgregory
In reply to this post by Rob Tompkins
On Thu, Aug 9, 2018 at 7:25 AM Rob Tompkins <[hidden email]> wrote:

>
>
> > On Aug 9, 2018, at 9:17 AM, Gilles <[hidden email]> wrote:
> >
> > On Thu, 9 Aug 2018 08:43:34 -0400, Rob Tompkins wrote:
> >>> On Aug 9, 2018, at 8:38 AM, Gilles <[hidden email]>
> wrote:
> >>>
> >>> On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote:
> >>>> Are we instead trying to remove the “extends” from all of those
> classes?
> >>>
> >>> I'm not sure what you mean, sorry.
> >>>
> >>> "SamplerBase" was meant has a utility/shortcut/boiler-plate in order
> >>> to copy the "rng" argument in one place, instead of having a field
> >>> in each sampler class. It is purely internal to this library (in C++,
> >>> inheritance would have been "private”).
> >>
> >> Are we trying to remove SamplerBase all together?
> >
> > That would break BC the same way.
> > I wouldn't mind since the shortcut is not really significant.
> >
> > But my reasoning would be the same: correct usage does not refer
> > to these methods so the fix could be done now, with minimal risk
> > (due to low usage of a new component).
> >
> >>>> My thought was to override the method and javadoc locally (in the
> >>>> actual subclass) so that we can give reasonable deprecation messages,
> >>>> but that’s only a thought. I’m ok with whatever.
> >
> > It's overkill in this context.
> > We shouldn't be rigid about the "no BC in minor release" rule;
>
> I’m a tad confused here. I thought that the whole point of everything that
> we do in terms of the way we version and name packages
> was specifically driven at the fact that we’re completely rigid about “no
> BC breakage in minor releases.”
>
> I’ve not been around the project long enough to know this with certainty,
> but that definitely is the vibe that I get. I would think that such RCs
> would generally get -1’s from people.
>
> Generally I’m open to whatever. I’m just operating on what I perceive to
> be precedent.
>
> @Gary - what’s the call here on the flexibility of BC incompatible changes
> in a minor release?
>

BC is binary and we should not break stacks. You really don't want to
create jar hell for our users. Think about deep transitive dependencies. An
end use or developer is not going to care 2 cents about "correct vs
incorrect API usage" when a component depends on this depends on that and
so on, ends up breaking his or her whole build because a low level
component has decided that breaking BC was OK.

Gary


> > enforcing it is worse for everybody:
> > * bona fide users will be forced to modify their source and
> >   recompile in order to benefit from the performance boost
> >   introduced in version 1.1, and
> > * bad usage (if it exists at all) could continue unsuspected.
> >
> >>> The message would aim at the wrong target: for developers of this
> >>> library, there is no deprecation (the boiler-plate code is there
> >>> to be used); for application developers, the class should not be
> >>> there to be used (hence: package private).
> >>
> >> Then, I guess, we should simply state that we want to eventually
> >> delete these methods and they should not be consumed. Yeah?
> >
> > No (cf. above); I propose to delete them *now*.
> >
> >> Like I
> >> said…I’m up for whatever. I’m just trying to balance yours and Gary’s
> >> points and find a good middle ground that everyone can live with.
> >
> > Which are the two sides for which you try to find a middle ground?
> > At any rate it would not be "good" if it makes developers and users
> > unhappy for a case that probably doesn't exist out there.
> > Tools such as Clirr are a great help, but they shouldn't induce us
> > to take the wrong course only to have the pleasure to contemplate a
> > clean report. ;-)
> > Thanks to Clirr (and Gary), we've become aware of a design issue.
> > We can decide to resolve it as it was done in the past, with IMHO
> > zero inconvenience (except for the Clirr "unclean" report).
> >
> > Regards,
> > Gilles
> >
> >> -Rob
> >>
> >>>
> >>> Gilles
> >>>
> >>>> -Rob
> >>>>
> >>>>> On Aug 9, 2018, at 7:02 AM, Gilles <[hidden email]>
> wrote:
> >>>>>
> >>>>> Hi all.
> >>>>>
> >>>>> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
> >>>>>> Hello Rob.
> >>>>>>
> >>>>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
> >>>>>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
> >>>>>>> only a +0 on this change. So, if you want to revert it before
> going up
> >>>>>>> with 1.1, that’s fine.
> >>>>>>
> >>>>>> I don't understand this change after what I answered to Gary's
> strange
> >>>>>> proposal.
> >>>>>> The comment is *wrong*.  As I said previously, the base class
> provides
> >>>>>> boiler-plate code; that is *not* deprecated.  The issue is that
> those
> >>>>>> methods were not meant to be used further down the hierarchy (in
> user's
> >>>>>> subclasses). [And now, furthermore, they are not used anymore in
> class
> >>>>>> "PoissonSampler", following the change in RNG-50 (delegation to
> other
> >>>>>> classes).]
> >>>>>> The sampler classes should have been made "final"; but now, this
> change
> >>>>>> would also be "breaking" (even though I doubt about legitimate
> use-cases
> >>>>>> for inherithing from the sampler implementations).
> >>>>>
> >>>>> Assuming it's unlikely that application developers would have
> >>>>> * called protected methods of "SamplerBase",[1]
> >>>>> * create subclasses of "SamplerBase",[1]
> >>>>> * create subclasses of the sampler classes,[1]
> >>>>> I suggest to
> >>>>> * make "SamplerBase" package private
> >>>>> * make the sampler clases "final".
> >>>>>
> >>>>> The above are the *less* intrusive fixes, as they would only
> >>>>> potentially impact application developers by making them aware
> >>>>> that they have made incorrect usage of an *inadvertently*
> >>>>> public API.  Correct usage will not be impacted even though
> >>>>> the change is not BC.[2]
> >>>>>
> >>>>> Any objection to that fix?[3]
> >>>>>
> >>>>> Regards,
> >>>>> Gilles
> >>>>>
> >>>>> [1] Rationale: (a) There is no reason to do that and
> >>>>>             (b) "Commons RNG" isn't much used at this point:
> >>>>>
> http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
> >>>>> [2] I recall that a similar situation (BC breaking in a minor
> >>>>>  release) occurred in CM, at a time where the number of
> >>>>>  potential users was much larger.
> >>>>> [3] I'll add a prominent warning in the changelog to the effect
> >>>>>  that people wanting to continue with incorrect usage of the
> >>>>>  samplers should not upgrade. ;-)
> >>>>>
> >>>>>>
> >>>>>> Please revert.
> >>>>>>
> >>>>>> Thanks,
> >>>>>> Gilles
> >>>>>>
> >>>>>>>
> >>>>>>> -Rob
> >>>>>>>
> >>>>>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
> >>>>>>>>
> >>>>>>>> Repository: commons-rng
> >>>>>>>> Updated Branches:
> >>>>>>>> refs/heads/1.1 50b984b1d -> f8159f28a
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Adding PoissonSampler deprecations. Use the correct faster public
> methods
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
> >>>>>>>> Commit:
> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
> >>>>>>>> Tree:
> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
> >>>>>>>> Diff:
> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
> >>>>>>>>
> >>>>>>>> Branch: refs/heads/1.1
> >>>>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
> >>>>>>>> Parents: 50b984b
> >>>>>>>> Author: Rob Tompkins <[hidden email]>
> >>>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
> >>>>>>>> Committer: Rob Tompkins <[hidden email]>
> >>>>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
> >>>>>>>>
> >>>>>>>>
> ----------------------------------------------------------------------
> >>>>>>>> .../sampling/distribution/PoissonSampler.java   | 42
> ++++++++++++++++++++
> >>>>>>>> 1 file changed, 42 insertions(+)
> >>>>>>>>
> ----------------------------------------------------------------------
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>
> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
> >>>>>>>>
> ----------------------------------------------------------------------
> >>>>>>>> diff --git
> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
> >>>>>>>> index d0733ba..29d0e4e 100644
> >>>>>>>> ---
> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
> >>>>>>>> +++
> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
> >>>>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
> >>>>>>>>      return poissonSampler.sample();
> >>>>>>>>  }
> >>>>>>>>
> >>>>>>>> +    /**
> >>>>>>>> +     * @return a random value from a uniform distribution in the
> >>>>>>>> +     * interval {@code [0, 1)}.
> >>>>>>>> +     * @deprecated - one should be using the {@link
> PoissonSampler#sample()} method,
> >>>>>>>> +     * as it is considerably faster.
> >>>>>>>> +     */
> >>>>>>>> +    @Deprecated
> >>>>>>>> +    protected double nextDouble() {
> >>>>>>>> +        return super.nextDouble();
> >>>>>>>> +    }
> >>>>>>>> +
> >>>>>>>> +    /**
> >>>>>>>> +     * @return a random {@code int} value.
> >>>>>>>> +     * @deprecated - one should be using the {@link
> PoissonSampler#sample()} method,
> >>>>>>>> +     * as it is considerably faster.
> >>>>>>>> +     */
> >>>>>>>> +    @Deprecated
> >>>>>>>> +    protected int nextInt() {
> >>>>>>>> +        return super.nextInt();
> >>>>>>>> +    }
> >>>>>>>> +
> >>>>>>>> +    /**
> >>>>>>>> +     * @param max Upper bound (excluded).
> >>>>>>>> +     * @return a random {@code int} value in the interval {@code
> [0, max)}.
> >>>>>>>> +     * @deprecated - one should be using the {@link
> PoissonSampler#sample()} method,
> >>>>>>>> +     *      * as it is considerably faster.
> >>>>>>>> +     */
> >>>>>>>> +    @Deprecated
> >>>>>>>> +    protected int nextInt(int max) {
> >>>>>>>> +        return super.nextInt(max);
> >>>>>>>> +    }
> >>>>>>>> +
> >>>>>>>> +    /**
> >>>>>>>> +     * @return a random {@code long} value.
> >>>>>>>> +     * @deprecated - one should be using the {@link
> PoissonSampler#sample()} method,
> >>>>>>>> +     *      * as it is considerably faster.
> >>>>>>>> +     */
> >>>>>>>> +    @Deprecated
> >>>>>>>> +    protected long nextLong() {
> >>>>>>>> +        return super.nextLong();
> >>>>>>>> +    }
> >>>>>>>> +
> >>>>>>>>  /** {@inheritDoc} */
> >>>>>>>>  @Override
> >>>>>>>>  public String toString() {
> >>>>>>>>
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

garydgregory
On Thu, Aug 9, 2018 at 8:47 AM Gary Gregory <[hidden email]> wrote:

>
>
> On Thu, Aug 9, 2018 at 7:25 AM Rob Tompkins <[hidden email]> wrote:
>
>>
>>
>> > On Aug 9, 2018, at 9:17 AM, Gilles <[hidden email]>
>> wrote:
>> >
>> > On Thu, 9 Aug 2018 08:43:34 -0400, Rob Tompkins wrote:
>> >>> On Aug 9, 2018, at 8:38 AM, Gilles <[hidden email]>
>> wrote:
>> >>>
>> >>> On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote:
>> >>>> Are we instead trying to remove the “extends” from all of those
>> classes?
>> >>>
>> >>> I'm not sure what you mean, sorry.
>> >>>
>> >>> "SamplerBase" was meant has a utility/shortcut/boiler-plate in order
>> >>> to copy the "rng" argument in one place, instead of having a field
>> >>> in each sampler class. It is purely internal to this library (in C++,
>> >>> inheritance would have been "private”).
>> >>
>> >> Are we trying to remove SamplerBase all together?
>> >
>> > That would break BC the same way.
>> > I wouldn't mind since the shortcut is not really significant.
>> >
>> > But my reasoning would be the same: correct usage does not refer
>> > to these methods so the fix could be done now, with minimal risk
>> > (due to low usage of a new component).
>> >
>> >>>> My thought was to override the method and javadoc locally (in the
>> >>>> actual subclass) so that we can give reasonable deprecation messages,
>> >>>> but that’s only a thought. I’m ok with whatever.
>> >
>> > It's overkill in this context.
>> > We shouldn't be rigid about the "no BC in minor release" rule;
>>
>> I’m a tad confused here. I thought that the whole point of everything
>> that we do in terms of the way we version and name packages
>> was specifically driven at the fact that we’re completely rigid about “no
>> BC breakage in minor releases.”
>>
>> I’ve not been around the project long enough to know this with certainty,
>> but that definitely is the vibe that I get. I would think that such RCs
>> would generally get -1’s from people.
>>
>> Generally I’m open to whatever. I’m just operating on what I perceive to
>> be precedent.
>>
>> @Gary - what’s the call here on the flexibility of BC incompatible
>> changes in a minor release?
>>
>
> BC is binary and we should not break stacks. You really don't want to
> create jar hell for our users. Think about deep transitive dependencies. An
> end use or developer is not going to care 2 cents about "correct vs
> incorrect API usage" when a component depends on this depends on that and
> so on, ends up breaking his or her whole build because a low level
> component has decided that breaking BC was OK.
>

I'll also note that I am not close enough to this component to opine on the
quality of the API or the workarounds discussed. My point is only "Don't
create jar hell; don't break BC."

Gary



>
> Gary
>
>
>> > enforcing it is worse for everybody:
>> > * bona fide users will be forced to modify their source and
>> >   recompile in order to benefit from the performance boost
>> >   introduced in version 1.1, and
>> > * bad usage (if it exists at all) could continue unsuspected.
>> >
>> >>> The message would aim at the wrong target: for developers of this
>> >>> library, there is no deprecation (the boiler-plate code is there
>> >>> to be used); for application developers, the class should not be
>> >>> there to be used (hence: package private).
>> >>
>> >> Then, I guess, we should simply state that we want to eventually
>> >> delete these methods and they should not be consumed. Yeah?
>> >
>> > No (cf. above); I propose to delete them *now*.
>> >
>> >> Like I
>> >> said…I’m up for whatever. I’m just trying to balance yours and Gary’s
>> >> points and find a good middle ground that everyone can live with.
>> >
>> > Which are the two sides for which you try to find a middle ground?
>> > At any rate it would not be "good" if it makes developers and users
>> > unhappy for a case that probably doesn't exist out there.
>> > Tools such as Clirr are a great help, but they shouldn't induce us
>> > to take the wrong course only to have the pleasure to contemplate a
>> > clean report. ;-)
>> > Thanks to Clirr (and Gary), we've become aware of a design issue.
>> > We can decide to resolve it as it was done in the past, with IMHO
>> > zero inconvenience (except for the Clirr "unclean" report).
>> >
>> > Regards,
>> > Gilles
>> >
>> >> -Rob
>> >>
>> >>>
>> >>> Gilles
>> >>>
>> >>>> -Rob
>> >>>>
>> >>>>> On Aug 9, 2018, at 7:02 AM, Gilles <[hidden email]>
>> wrote:
>> >>>>>
>> >>>>> Hi all.
>> >>>>>
>> >>>>> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
>> >>>>>> Hello Rob.
>> >>>>>>
>> >>>>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>> >>>>>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>> >>>>>>> only a +0 on this change. So, if you want to revert it before
>> going up
>> >>>>>>> with 1.1, that’s fine.
>> >>>>>>
>> >>>>>> I don't understand this change after what I answered to Gary's
>> strange
>> >>>>>> proposal.
>> >>>>>> The comment is *wrong*.  As I said previously, the base class
>> provides
>> >>>>>> boiler-plate code; that is *not* deprecated.  The issue is that
>> those
>> >>>>>> methods were not meant to be used further down the hierarchy (in
>> user's
>> >>>>>> subclasses). [And now, furthermore, they are not used anymore in
>> class
>> >>>>>> "PoissonSampler", following the change in RNG-50 (delegation to
>> other
>> >>>>>> classes).]
>> >>>>>> The sampler classes should have been made "final"; but now, this
>> change
>> >>>>>> would also be "breaking" (even though I doubt about legitimate
>> use-cases
>> >>>>>> for inherithing from the sampler implementations).
>> >>>>>
>> >>>>> Assuming it's unlikely that application developers would have
>> >>>>> * called protected methods of "SamplerBase",[1]
>> >>>>> * create subclasses of "SamplerBase",[1]
>> >>>>> * create subclasses of the sampler classes,[1]
>> >>>>> I suggest to
>> >>>>> * make "SamplerBase" package private
>> >>>>> * make the sampler clases "final".
>> >>>>>
>> >>>>> The above are the *less* intrusive fixes, as they would only
>> >>>>> potentially impact application developers by making them aware
>> >>>>> that they have made incorrect usage of an *inadvertently*
>> >>>>> public API.  Correct usage will not be impacted even though
>> >>>>> the change is not BC.[2]
>> >>>>>
>> >>>>> Any objection to that fix?[3]
>> >>>>>
>> >>>>> Regards,
>> >>>>> Gilles
>> >>>>>
>> >>>>> [1] Rationale: (a) There is no reason to do that and
>> >>>>>             (b) "Commons RNG" isn't much used at this point:
>> >>>>>
>> http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
>> >>>>> [2] I recall that a similar situation (BC breaking in a minor
>> >>>>>  release) occurred in CM, at a time where the number of
>> >>>>>  potential users was much larger.
>> >>>>> [3] I'll add a prominent warning in the changelog to the effect
>> >>>>>  that people wanting to continue with incorrect usage of the
>> >>>>>  samplers should not upgrade. ;-)
>> >>>>>
>> >>>>>>
>> >>>>>> Please revert.
>> >>>>>>
>> >>>>>> Thanks,
>> >>>>>> Gilles
>> >>>>>>
>> >>>>>>>
>> >>>>>>> -Rob
>> >>>>>>>
>> >>>>>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>> >>>>>>>>
>> >>>>>>>> Repository: commons-rng
>> >>>>>>>> Updated Branches:
>> >>>>>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>> Adding PoissonSampler deprecations. Use the correct faster
>> public methods
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>> >>>>>>>> Commit:
>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>> >>>>>>>> Tree:
>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>> >>>>>>>> Diff:
>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>> >>>>>>>>
>> >>>>>>>> Branch: refs/heads/1.1
>> >>>>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>> >>>>>>>> Parents: 50b984b
>> >>>>>>>> Author: Rob Tompkins <[hidden email]>
>> >>>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>> >>>>>>>> Committer: Rob Tompkins <[hidden email]>
>> >>>>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>> >>>>>>>>
>> >>>>>>>>
>> ----------------------------------------------------------------------
>> >>>>>>>> .../sampling/distribution/PoissonSampler.java   | 42
>> ++++++++++++++++++++
>> >>>>>>>> 1 file changed, 42 insertions(+)
>> >>>>>>>>
>> ----------------------------------------------------------------------
>> >>>>>>>>
>> >>>>>>>>
>> >>>>>>>>
>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>> >>>>>>>>
>> ----------------------------------------------------------------------
>> >>>>>>>> diff --git
>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>> >>>>>>>> index d0733ba..29d0e4e 100644
>> >>>>>>>> ---
>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>> >>>>>>>> +++
>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>> >>>>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>> >>>>>>>>      return poissonSampler.sample();
>> >>>>>>>>  }
>> >>>>>>>>
>> >>>>>>>> +    /**
>> >>>>>>>> +     * @return a random value from a uniform distribution in the
>> >>>>>>>> +     * interval {@code [0, 1)}.
>> >>>>>>>> +     * @deprecated - one should be using the {@link
>> PoissonSampler#sample()} method,
>> >>>>>>>> +     * as it is considerably faster.
>> >>>>>>>> +     */
>> >>>>>>>> +    @Deprecated
>> >>>>>>>> +    protected double nextDouble() {
>> >>>>>>>> +        return super.nextDouble();
>> >>>>>>>> +    }
>> >>>>>>>> +
>> >>>>>>>> +    /**
>> >>>>>>>> +     * @return a random {@code int} value.
>> >>>>>>>> +     * @deprecated - one should be using the {@link
>> PoissonSampler#sample()} method,
>> >>>>>>>> +     * as it is considerably faster.
>> >>>>>>>> +     */
>> >>>>>>>> +    @Deprecated
>> >>>>>>>> +    protected int nextInt() {
>> >>>>>>>> +        return super.nextInt();
>> >>>>>>>> +    }
>> >>>>>>>> +
>> >>>>>>>> +    /**
>> >>>>>>>> +     * @param max Upper bound (excluded).
>> >>>>>>>> +     * @return a random {@code int} value in the interval
>> {@code [0, max)}.
>> >>>>>>>> +     * @deprecated - one should be using the {@link
>> PoissonSampler#sample()} method,
>> >>>>>>>> +     *      * as it is considerably faster.
>> >>>>>>>> +     */
>> >>>>>>>> +    @Deprecated
>> >>>>>>>> +    protected int nextInt(int max) {
>> >>>>>>>> +        return super.nextInt(max);
>> >>>>>>>> +    }
>> >>>>>>>> +
>> >>>>>>>> +    /**
>> >>>>>>>> +     * @return a random {@code long} value.
>> >>>>>>>> +     * @deprecated - one should be using the {@link
>> PoissonSampler#sample()} method,
>> >>>>>>>> +     *      * as it is considerably faster.
>> >>>>>>>> +     */
>> >>>>>>>> +    @Deprecated
>> >>>>>>>> +    protected long nextLong() {
>> >>>>>>>> +        return super.nextLong();
>> >>>>>>>> +    }
>> >>>>>>>> +
>> >>>>>>>>  /** {@inheritDoc} */
>> >>>>>>>>  @Override
>> >>>>>>>>  public String toString() {
>> >>>>>>>>
>> >
>> >
>> > ---------------------------------------------------------------------
>> > To unsubscribe, e-mail: [hidden email]
>> > For additional commands, e-mail: [hidden email]
>> >
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

Gilles Sadowski
On Thu, 9 Aug 2018 08:49:26 -0600, Gary Gregory wrote:

> On Thu, Aug 9, 2018 at 8:47 AM Gary Gregory <[hidden email]>
> wrote:
>
>>
>>
>> On Thu, Aug 9, 2018 at 7:25 AM Rob Tompkins <[hidden email]>
>> wrote:
>>
>>>
>>>
>>> > On Aug 9, 2018, at 9:17 AM, Gilles <[hidden email]>
>>> wrote:
>>> >
>>> > On Thu, 9 Aug 2018 08:43:34 -0400, Rob Tompkins wrote:
>>> >>> On Aug 9, 2018, at 8:38 AM, Gilles
>>> <[hidden email]>
>>> wrote:
>>> >>>
>>> >>> On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote:
>>> >>>> Are we instead trying to remove the “extends” from all of
>>> those
>>> classes?
>>> >>>
>>> >>> I'm not sure what you mean, sorry.
>>> >>>
>>> >>> "SamplerBase" was meant has a utility/shortcut/boiler-plate in
>>> order
>>> >>> to copy the "rng" argument in one place, instead of having a
>>> field
>>> >>> in each sampler class. It is purely internal to this library
>>> (in C++,
>>> >>> inheritance would have been "private”).
>>> >>
>>> >> Are we trying to remove SamplerBase all together?
>>> >
>>> > That would break BC the same way.
>>> > I wouldn't mind since the shortcut is not really significant.
>>> >
>>> > But my reasoning would be the same: correct usage does not refer
>>> > to these methods so the fix could be done now, with minimal risk
>>> > (due to low usage of a new component).
>>> >
>>> >>>> My thought was to override the method and javadoc locally (in
>>> the
>>> >>>> actual subclass) so that we can give reasonable deprecation
>>> messages,
>>> >>>> but that’s only a thought. I’m ok with whatever.
>>> >
>>> > It's overkill in this context.
>>> > We shouldn't be rigid about the "no BC in minor release" rule;
>>>
>>> I’m a tad confused here. I thought that the whole point of
>>> everything
>>> that we do in terms of the way we version and name packages
>>> was specifically driven at the fact that we’re completely rigid
>>> about “no
>>> BC breakage in minor releases.”
>>>
>>> I’ve not been around the project long enough to know this with
>>> certainty,
>>> but that definitely is the vibe that I get. I would think that such
>>> RCs
>>> would generally get -1’s from people.
>>>
>>> Generally I’m open to whatever. I’m just operating on what I
>>> perceive to
>>> be precedent.
>>>
>>> @Gary - what’s the call here on the flexibility of BC incompatible
>>> changes in a minor release?
>>>
>>
>> BC is binary and we should not break stacks. You really don't want
>> to
>> create jar hell for our users. Think about deep transitive
>> dependencies. An
>> end use or developer is not going to care 2 cents about "correct vs
>> incorrect API usage" when a component depends on this depends on
>> that and
>> so on, ends up breaking his or her whole build because a low level
>> component has decided that breaking BC was OK.

Are you kidding?
Here, "wrong usage" implies wrong results.

Wrong usage is worse than JAR hell.
[But should it occur in this case, it will manifest itself by
giving wrong results (if the stack happened to select v1.0) or
raising a "VerifyError" (with v1.1 with the proposed changes).]

> I'll also note that I am not close enough to this component to opine
> on the
> quality of the API or the workarounds discussed. My point is only
> "Don't
> create jar hell; don't break BC."

On the one hand, you declare that you cannot discuss the issue
but on the other, you think that you know what is the worse of
two evils.
What about trusting that those "close enough to this component"
do propose the best solution in the current situation?

If we make the (BC breaking) changes, then
  * either no stack will be broken,
  * or the user will be warned that his code is computing invalid
    results.
If we don't make the changes, then
  * no stack will be broken, but
  * the user may unsuspectingly continue getting invalid results.

IMO that's a pretty easy choice.

Gilles

P.S. As a side note, we should all see now why I insisted on
      having the "core" code (RNG algorithms) in one component
      and the "application" code (a.o. samplers) in another: all
      the issues, delays, workarounds (and all the changes for
      v1.1) solely concern module "commons-rng-sampling".
      And because this application needs fixing, other applications
      out there, even those that do *not* use the samplers, would
      be required to modify their source code?

>
> Gary
>
>
>
>>
>> Gary
>>
>>
>>> > enforcing it is worse for everybody:
>>> > * bona fide users will be forced to modify their source and
>>> >   recompile in order to benefit from the performance boost
>>> >   introduced in version 1.1, and
>>> > * bad usage (if it exists at all) could continue unsuspected.
>>> >
>>> >>> The message would aim at the wrong target: for developers of
>>> this
>>> >>> library, there is no deprecation (the boiler-plate code is
>>> there
>>> >>> to be used); for application developers, the class should not
>>> be
>>> >>> there to be used (hence: package private).
>>> >>
>>> >> Then, I guess, we should simply state that we want to eventually
>>> >> delete these methods and they should not be consumed. Yeah?
>>> >
>>> > No (cf. above); I propose to delete them *now*.
>>> >
>>> >> Like I
>>> >> said…I’m up for whatever. I’m just trying to balance yours and
>>> Gary’s
>>> >> points and find a good middle ground that everyone can live
>>> with.
>>> >
>>> > Which are the two sides for which you try to find a middle
>>> ground?
>>> > At any rate it would not be "good" if it makes developers and
>>> users
>>> > unhappy for a case that probably doesn't exist out there.
>>> > Tools such as Clirr are a great help, but they shouldn't induce
>>> us
>>> > to take the wrong course only to have the pleasure to contemplate
>>> a
>>> > clean report. ;-)
>>> > Thanks to Clirr (and Gary), we've become aware of a design issue.
>>> > We can decide to resolve it as it was done in the past, with IMHO
>>> > zero inconvenience (except for the Clirr "unclean" report).
>>> >
>>> > Regards,
>>> > Gilles
>>> >
>>> >> -Rob
>>> >>
>>> >>>
>>> >>> Gilles
>>> >>>
>>> >>>> -Rob
>>> >>>>
>>> >>>>> On Aug 9, 2018, at 7:02 AM, Gilles
>>> <[hidden email]>
>>> wrote:
>>> >>>>>
>>> >>>>> Hi all.
>>> >>>>>
>>> >>>>> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
>>> >>>>>> Hello Rob.
>>> >>>>>>
>>> >>>>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>> >>>>>>> @Gilles - thoughts here?? Just kinda what I was thinking,
>>> but I’m
>>> >>>>>>> only a +0 on this change. So, if you want to revert it
>>> before
>>> going up
>>> >>>>>>> with 1.1, that’s fine.
>>> >>>>>>
>>> >>>>>> I don't understand this change after what I answered to
>>> Gary's
>>> strange
>>> >>>>>> proposal.
>>> >>>>>> The comment is *wrong*.  As I said previously, the base
>>> class
>>> provides
>>> >>>>>> boiler-plate code; that is *not* deprecated.  The issue is
>>> that
>>> those
>>> >>>>>> methods were not meant to be used further down the hierarchy
>>> (in
>>> user's
>>> >>>>>> subclasses). [And now, furthermore, they are not used
>>> anymore in
>>> class
>>> >>>>>> "PoissonSampler", following the change in RNG-50 (delegation
>>> to
>>> other
>>> >>>>>> classes).]
>>> >>>>>> The sampler classes should have been made "final"; but now,
>>> this
>>> change
>>> >>>>>> would also be "breaking" (even though I doubt about
>>> legitimate
>>> use-cases
>>> >>>>>> for inherithing from the sampler implementations).
>>> >>>>>
>>> >>>>> Assuming it's unlikely that application developers would have
>>> >>>>> * called protected methods of "SamplerBase",[1]
>>> >>>>> * create subclasses of "SamplerBase",[1]
>>> >>>>> * create subclasses of the sampler classes,[1]
>>> >>>>> I suggest to
>>> >>>>> * make "SamplerBase" package private
>>> >>>>> * make the sampler clases "final".
>>> >>>>>
>>> >>>>> The above are the *less* intrusive fixes, as they would only
>>> >>>>> potentially impact application developers by making them
>>> aware
>>> >>>>> that they have made incorrect usage of an *inadvertently*
>>> >>>>> public API.  Correct usage will not be impacted even though
>>> >>>>> the change is not BC.[2]
>>> >>>>>
>>> >>>>> Any objection to that fix?[3]
>>> >>>>>
>>> >>>>> Regards,
>>> >>>>> Gilles
>>> >>>>>
>>> >>>>> [1] Rationale: (a) There is no reason to do that and
>>> >>>>>             (b) "Commons RNG" isn't much used at this point:
>>> >>>>>
>>>
>>> http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
>>> >>>>> [2] I recall that a similar situation (BC breaking in a minor
>>> >>>>>  release) occurred in CM, at a time where the number of
>>> >>>>>  potential users was much larger.
>>> >>>>> [3] I'll add a prominent warning in the changelog to the
>>> effect
>>> >>>>>  that people wanting to continue with incorrect usage of the
>>> >>>>>  samplers should not upgrade. ;-)
>>> >>>>>
>>> >>>>>>
>>> >>>>>> Please revert.
>>> >>>>>>
>>> >>>>>> Thanks,
>>> >>>>>> Gilles
>>> >>>>>>
>>> >>>>>>>
>>> >>>>>>> -Rob
>>> >>>>>>>
>>> >>>>>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>> >>>>>>>>
>>> >>>>>>>> Repository: commons-rng
>>> >>>>>>>> Updated Branches:
>>> >>>>>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>> >>>>>>>>
>>> >>>>>>>>
>>> >>>>>>>> Adding PoissonSampler deprecations. Use the correct faster
>>> public methods
>>> >>>>>>>>
>>> >>>>>>>>
>>> >>>>>>>> Project:
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>> >>>>>>>> Commit:
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>> >>>>>>>> Tree:
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>> >>>>>>>> Diff:
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>> >>>>>>>>
>>> >>>>>>>> Branch: refs/heads/1.1
>>> >>>>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>> >>>>>>>> Parents: 50b984b
>>> >>>>>>>> Author: Rob Tompkins <[hidden email]>
>>> >>>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>> >>>>>>>> Committer: Rob Tompkins <[hidden email]>
>>> >>>>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>> >>>>>>>>
>>> >>>>>>>>
>>>
>>> ----------------------------------------------------------------------
>>> >>>>>>>> .../sampling/distribution/PoissonSampler.java   | 42
>>> ++++++++++++++++++++
>>> >>>>>>>> 1 file changed, 42 insertions(+)
>>> >>>>>>>>
>>>
>>> ----------------------------------------------------------------------
>>> >>>>>>>>
>>> >>>>>>>>
>>> >>>>>>>>
>>>
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> >>>>>>>>
>>>
>>> ----------------------------------------------------------------------
>>> >>>>>>>> diff --git
>>>
>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>
>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> >>>>>>>> index d0733ba..29d0e4e 100644
>>> >>>>>>>> ---
>>>
>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> >>>>>>>> +++
>>>
>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> >>>>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>> >>>>>>>>      return poissonSampler.sample();
>>> >>>>>>>>  }
>>> >>>>>>>>
>>> >>>>>>>> +    /**
>>> >>>>>>>> +     * @return a random value from a uniform distribution
>>> in the
>>> >>>>>>>> +     * interval {@code [0, 1)}.
>>> >>>>>>>> +     * @deprecated - one should be using the {@link
>>> PoissonSampler#sample()} method,
>>> >>>>>>>> +     * as it is considerably faster.
>>> >>>>>>>> +     */
>>> >>>>>>>> +    @Deprecated
>>> >>>>>>>> +    protected double nextDouble() {
>>> >>>>>>>> +        return super.nextDouble();
>>> >>>>>>>> +    }
>>> >>>>>>>> +
>>> >>>>>>>> +    /**
>>> >>>>>>>> +     * @return a random {@code int} value.
>>> >>>>>>>> +     * @deprecated - one should be using the {@link
>>> PoissonSampler#sample()} method,
>>> >>>>>>>> +     * as it is considerably faster.
>>> >>>>>>>> +     */
>>> >>>>>>>> +    @Deprecated
>>> >>>>>>>> +    protected int nextInt() {
>>> >>>>>>>> +        return super.nextInt();
>>> >>>>>>>> +    }
>>> >>>>>>>> +
>>> >>>>>>>> +    /**
>>> >>>>>>>> +     * @param max Upper bound (excluded).
>>> >>>>>>>> +     * @return a random {@code int} value in the interval
>>> {@code [0, max)}.
>>> >>>>>>>> +     * @deprecated - one should be using the {@link
>>> PoissonSampler#sample()} method,
>>> >>>>>>>> +     *      * as it is considerably faster.
>>> >>>>>>>> +     */
>>> >>>>>>>> +    @Deprecated
>>> >>>>>>>> +    protected int nextInt(int max) {
>>> >>>>>>>> +        return super.nextInt(max);
>>> >>>>>>>> +    }
>>> >>>>>>>> +
>>> >>>>>>>> +    /**
>>> >>>>>>>> +     * @return a random {@code long} value.
>>> >>>>>>>> +     * @deprecated - one should be using the {@link
>>> PoissonSampler#sample()} method,
>>> >>>>>>>> +     *      * as it is considerably faster.
>>> >>>>>>>> +     */
>>> >>>>>>>> +    @Deprecated
>>> >>>>>>>> +    protected long nextLong() {
>>> >>>>>>>> +        return super.nextLong();
>>> >>>>>>>> +    }
>>> >>>>>>>> +
>>> >>>>>>>>  /** {@inheritDoc} */
>>> >>>>>>>>  @Override
>>> >>>>>>>>  public String toString() {
>>> >>>>>>>>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

Rob Tompkins
In reply to this post by garydgregory
I think we can make your changes Gilles, if we do a 2.0. Would you prefer to go that route? If we go with 1.1 (other than via deprecation), I think we’re stuck behind a bunch of -1 votes, and Id rather not roll an RC to that audience. I can work on the BC changes or the version changes between now and next week.

-Rob

> On Aug 9, 2018, at 10:49 AM, Gary Gregory <[hidden email]> wrote:
>
>> On Thu, Aug 9, 2018 at 8:47 AM Gary Gregory <[hidden email]> wrote:
>>
>>
>>
>>> On Thu, Aug 9, 2018 at 7:25 AM Rob Tompkins <[hidden email]> wrote:
>>>
>>>
>>>
>>>> On Aug 9, 2018, at 9:17 AM, Gilles <[hidden email]>
>>> wrote:
>>>>
>>>> On Thu, 9 Aug 2018 08:43:34 -0400, Rob Tompkins wrote:
>>>>>> On Aug 9, 2018, at 8:38 AM, Gilles <[hidden email]>
>>> wrote:
>>>>>>
>>>>>>> On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote:
>>>>>>> Are we instead trying to remove the “extends” from all of those
>>> classes?
>>>>>>
>>>>>> I'm not sure what you mean, sorry.
>>>>>>
>>>>>> "SamplerBase" was meant has a utility/shortcut/boiler-plate in order
>>>>>> to copy the "rng" argument in one place, instead of having a field
>>>>>> in each sampler class. It is purely internal to this library (in C++,
>>>>>> inheritance would have been "private”).
>>>>>
>>>>> Are we trying to remove SamplerBase all together?
>>>>
>>>> That would break BC the same way.
>>>> I wouldn't mind since the shortcut is not really significant.
>>>>
>>>> But my reasoning would be the same: correct usage does not refer
>>>> to these methods so the fix could be done now, with minimal risk
>>>> (due to low usage of a new component).
>>>>
>>>>>>> My thought was to override the method and javadoc locally (in the
>>>>>>> actual subclass) so that we can give reasonable deprecation messages,
>>>>>>> but that’s only a thought. I’m ok with whatever.
>>>>
>>>> It's overkill in this context.
>>>> We shouldn't be rigid about the "no BC in minor release" rule;
>>>
>>> I’m a tad confused here. I thought that the whole point of everything
>>> that we do in terms of the way we version and name packages
>>> was specifically driven at the fact that we’re completely rigid about “no
>>> BC breakage in minor releases.”
>>>
>>> I’ve not been around the project long enough to know this with certainty,
>>> but that definitely is the vibe that I get. I would think that such RCs
>>> would generally get -1’s from people.
>>>
>>> Generally I’m open to whatever. I’m just operating on what I perceive to
>>> be precedent.
>>>
>>> @Gary - what’s the call here on the flexibility of BC incompatible
>>> changes in a minor release?
>>>
>>
>> BC is binary and we should not break stacks. You really don't want to
>> create jar hell for our users. Think about deep transitive dependencies. An
>> end use or developer is not going to care 2 cents about "correct vs
>> incorrect API usage" when a component depends on this depends on that and
>> so on, ends up breaking his or her whole build because a low level
>> component has decided that breaking BC was OK.
>>
>
> I'll also note that I am not close enough to this component to opine on the
> quality of the API or the workarounds discussed. My point is only "Don't
> create jar hell; don't break BC."
>
> Gary
>
>
>
>>
>> Gary
>>
>>
>>>> enforcing it is worse for everybody:
>>>> * bona fide users will be forced to modify their source and
>>>>  recompile in order to benefit from the performance boost
>>>>  introduced in version 1.1, and
>>>> * bad usage (if it exists at all) could continue unsuspected.
>>>>
>>>>>> The message would aim at the wrong target: for developers of this
>>>>>> library, there is no deprecation (the boiler-plate code is there
>>>>>> to be used); for application developers, the class should not be
>>>>>> there to be used (hence: package private).
>>>>>
>>>>> Then, I guess, we should simply state that we want to eventually
>>>>> delete these methods and they should not be consumed. Yeah?
>>>>
>>>> No (cf. above); I propose to delete them *now*.
>>>>
>>>>> Like I
>>>>> said…I’m up for whatever. I’m just trying to balance yours and Gary’s
>>>>> points and find a good middle ground that everyone can live with.
>>>>
>>>> Which are the two sides for which you try to find a middle ground?
>>>> At any rate it would not be "good" if it makes developers and users
>>>> unhappy for a case that probably doesn't exist out there.
>>>> Tools such as Clirr are a great help, but they shouldn't induce us
>>>> to take the wrong course only to have the pleasure to contemplate a
>>>> clean report. ;-)
>>>> Thanks to Clirr (and Gary), we've become aware of a design issue.
>>>> We can decide to resolve it as it was done in the past, with IMHO
>>>> zero inconvenience (except for the Clirr "unclean" report).
>>>>
>>>> Regards,
>>>> Gilles
>>>>
>>>>> -Rob
>>>>>
>>>>>>
>>>>>> Gilles
>>>>>>
>>>>>>> -Rob
>>>>>>>
>>>>>>>> On Aug 9, 2018, at 7:02 AM, Gilles <[hidden email]>
>>> wrote:
>>>>>>>>
>>>>>>>> Hi all.
>>>>>>>>
>>>>>>>>> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
>>>>>>>>> Hello Rob.
>>>>>>>>>
>>>>>>>>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>>>>>>>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>>>>>>>>>> only a +0 on this change. So, if you want to revert it before
>>> going up
>>>>>>>>>> with 1.1, that’s fine.
>>>>>>>>>
>>>>>>>>> I don't understand this change after what I answered to Gary's
>>> strange
>>>>>>>>> proposal.
>>>>>>>>> The comment is *wrong*.  As I said previously, the base class
>>> provides
>>>>>>>>> boiler-plate code; that is *not* deprecated.  The issue is that
>>> those
>>>>>>>>> methods were not meant to be used further down the hierarchy (in
>>> user's
>>>>>>>>> subclasses). [And now, furthermore, they are not used anymore in
>>> class
>>>>>>>>> "PoissonSampler", following the change in RNG-50 (delegation to
>>> other
>>>>>>>>> classes).]
>>>>>>>>> The sampler classes should have been made "final"; but now, this
>>> change
>>>>>>>>> would also be "breaking" (even though I doubt about legitimate
>>> use-cases
>>>>>>>>> for inherithing from the sampler implementations).
>>>>>>>>
>>>>>>>> Assuming it's unlikely that application developers would have
>>>>>>>> * called protected methods of "SamplerBase",[1]
>>>>>>>> * create subclasses of "SamplerBase",[1]
>>>>>>>> * create subclasses of the sampler classes,[1]
>>>>>>>> I suggest to
>>>>>>>> * make "SamplerBase" package private
>>>>>>>> * make the sampler clases "final".
>>>>>>>>
>>>>>>>> The above are the *less* intrusive fixes, as they would only
>>>>>>>> potentially impact application developers by making them aware
>>>>>>>> that they have made incorrect usage of an *inadvertently*
>>>>>>>> public API.  Correct usage will not be impacted even though
>>>>>>>> the change is not BC.[2]
>>>>>>>>
>>>>>>>> Any objection to that fix?[3]
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Gilles
>>>>>>>>
>>>>>>>> [1] Rationale: (a) There is no reason to do that and
>>>>>>>>            (b) "Commons RNG" isn't much used at this point:
>>>>>>>>
>>> http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
>>>>>>>> [2] I recall that a similar situation (BC breaking in a minor
>>>>>>>> release) occurred in CM, at a time where the number of
>>>>>>>> potential users was much larger.
>>>>>>>> [3] I'll add a prominent warning in the changelog to the effect
>>>>>>>> that people wanting to continue with incorrect usage of the
>>>>>>>> samplers should not upgrade. ;-)
>>>>>>>>
>>>>>>>>>
>>>>>>>>> Please revert.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Gilles
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -Rob
>>>>>>>>>>
>>>>>>>>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>>>>>>>>>
>>>>>>>>>>> Repository: commons-rng
>>>>>>>>>>> Updated Branches:
>>>>>>>>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Adding PoissonSampler deprecations. Use the correct faster
>>> public methods
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>>>>>>>>> Commit:
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>>>>>>>>> Tree:
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>>>>>>>>> Diff:
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>>>>>>>>>
>>>>>>>>>>> Branch: refs/heads/1.1
>>>>>>>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>>>>>>>>> Parents: 50b984b
>>>>>>>>>>> Author: Rob Tompkins <[hidden email]>
>>>>>>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>>>>>>>>> Committer: Rob Tompkins <[hidden email]>
>>>>>>>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>>>>>>>>>
>>>>>>>>>>>
>>> ----------------------------------------------------------------------
>>>>>>>>>>> .../sampling/distribution/PoissonSampler.java   | 42
>>> ++++++++++++++++++++
>>>>>>>>>>> 1 file changed, 42 insertions(+)
>>>>>>>>>>>
>>> ----------------------------------------------------------------------
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>>>>
>>> ----------------------------------------------------------------------
>>>>>>>>>>> diff --git
>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>>>> index d0733ba..29d0e4e 100644
>>>>>>>>>>> ---
>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>>>> +++
>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>>>>>>>>     return poissonSampler.sample();
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>> +    /**
>>>>>>>>>>> +     * @return a random value from a uniform distribution in the
>>>>>>>>>>> +     * interval {@code [0, 1)}.
>>>>>>>>>>> +     * @deprecated - one should be using the {@link
>>> PoissonSampler#sample()} method,
>>>>>>>>>>> +     * as it is considerably faster.
>>>>>>>>>>> +     */
>>>>>>>>>>> +    @Deprecated
>>>>>>>>>>> +    protected double nextDouble() {
>>>>>>>>>>> +        return super.nextDouble();
>>>>>>>>>>> +    }
>>>>>>>>>>> +
>>>>>>>>>>> +    /**
>>>>>>>>>>> +     * @return a random {@code int} value.
>>>>>>>>>>> +     * @deprecated - one should be using the {@link
>>> PoissonSampler#sample()} method,
>>>>>>>>>>> +     * as it is considerably faster.
>>>>>>>>>>> +     */
>>>>>>>>>>> +    @Deprecated
>>>>>>>>>>> +    protected int nextInt() {
>>>>>>>>>>> +        return super.nextInt();
>>>>>>>>>>> +    }
>>>>>>>>>>> +
>>>>>>>>>>> +    /**
>>>>>>>>>>> +     * @param max Upper bound (excluded).
>>>>>>>>>>> +     * @return a random {@code int} value in the interval
>>> {@code [0, max)}.
>>>>>>>>>>> +     * @deprecated - one should be using the {@link
>>> PoissonSampler#sample()} method,
>>>>>>>>>>> +     *      * as it is considerably faster.
>>>>>>>>>>> +     */
>>>>>>>>>>> +    @Deprecated
>>>>>>>>>>> +    protected int nextInt(int max) {
>>>>>>>>>>> +        return super.nextInt(max);
>>>>>>>>>>> +    }
>>>>>>>>>>> +
>>>>>>>>>>> +    /**
>>>>>>>>>>> +     * @return a random {@code long} value.
>>>>>>>>>>> +     * @deprecated - one should be using the {@link
>>> PoissonSampler#sample()} method,
>>>>>>>>>>> +     *      * as it is considerably faster.
>>>>>>>>>>> +     */
>>>>>>>>>>> +    @Deprecated
>>>>>>>>>>> +    protected long nextLong() {
>>>>>>>>>>> +        return super.nextLong();
>>>>>>>>>>> +    }
>>>>>>>>>>> +
>>>>>>>>>>> /** {@inheritDoc} */
>>>>>>>>>>> @Override
>>>>>>>>>>> public String toString() {
>>>>>>>>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: [hidden email]
>>>> For additional commands, e-mail: [hidden email]
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [hidden email]
>>> For additional commands, e-mail: [hidden email]
>>>
>>>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])

Gilles Sadowski
On Thu, 9 Aug 2018 17:16:01 -0400, Rob Tompkins wrote:
> I think we can make your changes Gilles, if we do a 2.0.

I know.

> Would you
> prefer to go that route?

Cure would be worse than the disease, for the reasons evoked.

Would it be possible to discuss *all* the arguments rather than
preemptively block the release as if a presumptive "JAR hell"
were the alpha and omega of reviewing?

> If we go with 1.1 (other than via
> deprecation), I think we’re stuck behind a bunch of -1 votes, and Id
> rather not roll an RC to that audience.

We discussed a similar issue a few months ago; and we agreed
about some BC breaking, provided it is documented in the
release notes.

There are 5 projects using "Commons RNG", 2 of them depend on the
"commons-rng-sampling" module and none would be impacted by a BC
breaking release.  No JAR hell here.
If v1.1 is released, any new project will use it.  No JAR hell
there.
Please enlighten me on how JAR hell could arise from the proposed
changes.


Regards,
Gilles

> I can work on the BC changes
> or the version changes between now and next week.
>
> -Rob
>
>> On Aug 9, 2018, at 10:49 AM, Gary Gregory <[hidden email]>
>> wrote:
>>
>>> On Thu, Aug 9, 2018 at 8:47 AM Gary Gregory
>>> <[hidden email]> wrote:
>>>
>>>
>>>
>>>> On Thu, Aug 9, 2018 at 7:25 AM Rob Tompkins <[hidden email]>
>>>> wrote:
>>>>
>>>>
>>>>
>>>>> On Aug 9, 2018, at 9:17 AM, Gilles <[hidden email]>
>>>> wrote:
>>>>>
>>>>> On Thu, 9 Aug 2018 08:43:34 -0400, Rob Tompkins wrote:
>>>>>>> On Aug 9, 2018, at 8:38 AM, Gilles
>>>>>>> <[hidden email]>
>>>> wrote:
>>>>>>>
>>>>>>>> On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote:
>>>>>>>> Are we instead trying to remove the “extends” from all of
>>>>>>>> those
>>>> classes?
>>>>>>>
>>>>>>> I'm not sure what you mean, sorry.
>>>>>>>
>>>>>>> "SamplerBase" was meant has a utility/shortcut/boiler-plate in
>>>>>>> order
>>>>>>> to copy the "rng" argument in one place, instead of having a
>>>>>>> field
>>>>>>> in each sampler class. It is purely internal to this library
>>>>>>> (in C++,
>>>>>>> inheritance would have been "private”).
>>>>>>
>>>>>> Are we trying to remove SamplerBase all together?
>>>>>
>>>>> That would break BC the same way.
>>>>> I wouldn't mind since the shortcut is not really significant.
>>>>>
>>>>> But my reasoning would be the same: correct usage does not refer
>>>>> to these methods so the fix could be done now, with minimal risk
>>>>> (due to low usage of a new component).
>>>>>
>>>>>>>> My thought was to override the method and javadoc locally (in
>>>>>>>> the
>>>>>>>> actual subclass) so that we can give reasonable deprecation
>>>>>>>> messages,
>>>>>>>> but that’s only a thought. I’m ok with whatever.
>>>>>
>>>>> It's overkill in this context.
>>>>> We shouldn't be rigid about the "no BC in minor release" rule;
>>>>
>>>> I’m a tad confused here. I thought that the whole point of
>>>> everything
>>>> that we do in terms of the way we version and name packages
>>>> was specifically driven at the fact that we’re completely rigid
>>>> about “no
>>>> BC breakage in minor releases.”
>>>>
>>>> I’ve not been around the project long enough to know this with
>>>> certainty,
>>>> but that definitely is the vibe that I get. I would think that
>>>> such RCs
>>>> would generally get -1’s from people.
>>>>
>>>> Generally I’m open to whatever. I’m just operating on what I
>>>> perceive to
>>>> be precedent.
>>>>
>>>> @Gary - what’s the call here on the flexibility of BC incompatible
>>>> changes in a minor release?
>>>>
>>>
>>> BC is binary and we should not break stacks. You really don't want
>>> to
>>> create jar hell for our users. Think about deep transitive
>>> dependencies. An
>>> end use or developer is not going to care 2 cents about "correct vs
>>> incorrect API usage" when a component depends on this depends on
>>> that and
>>> so on, ends up breaking his or her whole build because a low level
>>> component has decided that breaking BC was OK.
>>>
>>
>> I'll also note that I am not close enough to this component to opine
>> on the
>> quality of the API or the workarounds discussed. My point is only
>> "Don't
>> create jar hell; don't break BC."
>>
>> Gary
>>
>>
>>
>>>
>>> Gary
>>>
>>>
>>>>> enforcing it is worse for everybody:
>>>>> * bona fide users will be forced to modify their source and
>>>>>  recompile in order to benefit from the performance boost
>>>>>  introduced in version 1.1, and
>>>>> * bad usage (if it exists at all) could continue unsuspected.
>>>>>
>>>>>>> The message would aim at the wrong target: for developers of
>>>>>>> this
>>>>>>> library, there is no deprecation (the boiler-plate code is
>>>>>>> there
>>>>>>> to be used); for application developers, the class should not
>>>>>>> be
>>>>>>> there to be used (hence: package private).
>>>>>>
>>>>>> Then, I guess, we should simply state that we want to eventually
>>>>>> delete these methods and they should not be consumed. Yeah?
>>>>>
>>>>> No (cf. above); I propose to delete them *now*.
>>>>>
>>>>>> Like I
>>>>>> said…I’m up for whatever. I’m just trying to balance yours and
>>>>>> Gary’s
>>>>>> points and find a good middle ground that everyone can live
>>>>>> with.
>>>>>
>>>>> Which are the two sides for which you try to find a middle
>>>>> ground?
>>>>> At any rate it would not be "good" if it makes developers and
>>>>> users
>>>>> unhappy for a case that probably doesn't exist out there.
>>>>> Tools such as Clirr are a great help, but they shouldn't induce
>>>>> us
>>>>> to take the wrong course only to have the pleasure to contemplate
>>>>> a
>>>>> clean report. ;-)
>>>>> Thanks to Clirr (and Gary), we've become aware of a design issue.
>>>>> We can decide to resolve it as it was done in the past, with IMHO
>>>>> zero inconvenience (except for the Clirr "unclean" report).
>>>>>
>>>>> Regards,
>>>>> Gilles
>>>>>
>>>>>> -Rob
>>>>>>
>>>>>>>
>>>>>>> Gilles
>>>>>>>
>>>>>>>> -Rob
>>>>>>>>
>>>>>>>>> On Aug 9, 2018, at 7:02 AM, Gilles
>>>>>>>>> <[hidden email]>
>>>> wrote:
>>>>>>>>>
>>>>>>>>> Hi all.
>>>>>>>>>
>>>>>>>>>> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
>>>>>>>>>> Hello Rob.
>>>>>>>>>>
>>>>>>>>>>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>>>>>>>>>> @Gilles - thoughts here?? Just kinda what I was thinking,
>>>>>>>>>>> but I’m
>>>>>>>>>>> only a +0 on this change. So, if you want to revert it
>>>>>>>>>>> before
>>>> going up
>>>>>>>>>>> with 1.1, that’s fine.
>>>>>>>>>>
>>>>>>>>>> I don't understand this change after what I answered to
>>>>>>>>>> Gary's
>>>> strange
>>>>>>>>>> proposal.
>>>>>>>>>> The comment is *wrong*.  As I said previously, the base
>>>>>>>>>> class
>>>> provides
>>>>>>>>>> boiler-plate code; that is *not* deprecated.  The issue is
>>>>>>>>>> that
>>>> those
>>>>>>>>>> methods were not meant to be used further down the hierarchy
>>>>>>>>>> (in
>>>> user's
>>>>>>>>>> subclasses). [And now, furthermore, they are not used
>>>>>>>>>> anymore in
>>>> class
>>>>>>>>>> "PoissonSampler", following the change in RNG-50 (delegation
>>>>>>>>>> to
>>>> other
>>>>>>>>>> classes).]
>>>>>>>>>> The sampler classes should have been made "final"; but now,
>>>>>>>>>> this
>>>> change
>>>>>>>>>> would also be "breaking" (even though I doubt about
>>>>>>>>>> legitimate
>>>> use-cases
>>>>>>>>>> for inherithing from the sampler implementations).
>>>>>>>>>
>>>>>>>>> Assuming it's unlikely that application developers would have
>>>>>>>>> * called protected methods of "SamplerBase",[1]
>>>>>>>>> * create subclasses of "SamplerBase",[1]
>>>>>>>>> * create subclasses of the sampler classes,[1]
>>>>>>>>> I suggest to
>>>>>>>>> * make "SamplerBase" package private
>>>>>>>>> * make the sampler clases "final".
>>>>>>>>>
>>>>>>>>> The above are the *less* intrusive fixes, as they would only
>>>>>>>>> potentially impact application developers by making them
>>>>>>>>> aware
>>>>>>>>> that they have made incorrect usage of an *inadvertently*
>>>>>>>>> public API.  Correct usage will not be impacted even though
>>>>>>>>> the change is not BC.[2]
>>>>>>>>>
>>>>>>>>> Any objection to that fix?[3]
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Gilles
>>>>>>>>>
>>>>>>>>> [1] Rationale: (a) There is no reason to do that and
>>>>>>>>>            (b) "Commons RNG" isn't much used at this point:
>>>>>>>>>
>>>>
>>>> http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
>>>>>>>>> [2] I recall that a similar situation (BC breaking in a minor
>>>>>>>>> release) occurred in CM, at a time where the number of
>>>>>>>>> potential users was much larger.
>>>>>>>>> [3] I'll add a prominent warning in the changelog to the
>>>>>>>>> effect
>>>>>>>>> that people wanting to continue with incorrect usage of the
>>>>>>>>> samplers should not upgrade. ;-)
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Please revert.
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Gilles
>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> -Rob
>>>>>>>>>>>
>>>>>>>>>>>> On Aug 8, 2018, at 12:42 PM, [hidden email] wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Repository: commons-rng
>>>>>>>>>>>> Updated Branches:
>>>>>>>>>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Adding PoissonSampler deprecations. Use the correct faster
>>>> public methods
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>> Project:
>>>>>>>>>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>>>>>>>>>> Commit:
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>>>>>>>>>> Tree:
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>>>>>>>>>> Diff:
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>>>>>>>>>>
>>>>>>>>>>>> Branch: refs/heads/1.1
>>>>>>>>>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>>>>>>>>>> Parents: 50b984b
>>>>>>>>>>>> Author: Rob Tompkins <[hidden email]>
>>>>>>>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>>>>>>>>>> Committer: Rob Tompkins <[hidden email]>
>>>>>>>>>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>>>>>>>>>> .../sampling/distribution/PoissonSampler.java   | 42
>>>> ++++++++++++++++++++
>>>>>>>>>>>> 1 file changed, 42 insertions(+)
>>>>>>>>>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>>>>>
>>>>
>>>> ----------------------------------------------------------------------
>>>>>>>>>>>> diff --git
>>>>
>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>
>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>>>>> index d0733ba..29d0e4e 100644
>>>>>>>>>>>> ---
>>>>
>>>> a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>>>>> +++
>>>>
>>>> b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>>>>>>>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>>>>>>>>>     return poissonSampler.sample();
>>>>>>>>>>>> }
>>>>>>>>>>>>
>>>>>>>>>>>> +    /**
>>>>>>>>>>>> +     * @return a random value from a uniform distribution
>>>>>>>>>>>> in the
>>>>>>>>>>>> +     * interval {@code [0, 1)}.
>>>>>>>>>>>> +     * @deprecated - one should be using the {@link
>>>> PoissonSampler#sample()} method,
>>>>>>>>>>>> +     * as it is considerably faster.
>>>>>>>>>>>> +     */
>>>>>>>>>>>> +    @Deprecated
>>>>>>>>>>>> +    protected double nextDouble() {
>>>>>>>>>>>> +        return super.nextDouble();
>>>>>>>>>>>> +    }
>>>>>>>>>>>> +
>>>>>>>>>>>> +    /**
>>>>>>>>>>>> +     * @return a random {@code int} value.
>>>>>>>>>>>> +     * @deprecated - one should be using the {@link
>>>> PoissonSampler#sample()} method,
>>>>>>>>>>>> +     * as it is considerably faster.
>>>>>>>>>>>> +     */
>>>>>>>>>>>> +    @Deprecated
>>>>>>>>>>>> +    protected int nextInt() {
>>>>>>>>>>>> +        return super.nextInt();
>>>>>>>>>>>> +    }
>>>>>>>>>>>> +
>>>>>>>>>>>> +    /**
>>>>>>>>>>>> +     * @param max Upper bound (excluded).
>>>>>>>>>>>> +     * @return a random {@code int} value in the interval
>>>> {@code [0, max)}.
>>>>>>>>>>>> +     * @deprecated - one should be using the {@link
>>>> PoissonSampler#sample()} method,
>>>>>>>>>>>> +     *      * as it is considerably faster.
>>>>>>>>>>>> +     */
>>>>>>>>>>>> +    @Deprecated
>>>>>>>>>>>> +    protected int nextInt(int max) {
>>>>>>>>>>>> +        return super.nextInt(max);
>>>>>>>>>>>> +    }
>>>>>>>>>>>> +
>>>>>>>>>>>> +    /**
>>>>>>>>>>>> +     * @return a random {@code long} value.
>>>>>>>>>>>> +     * @deprecated - one should be using the {@link
>>>> PoissonSampler#sample()} method,
>>>>>>>>>>>> +     *      * as it is considerably faster.
>>>>>>>>>>>> +     */
>>>>>>>>>>>> +    @Deprecated
>>>>>>>>>>>> +    protected long nextLong() {
>>>>>>>>>>>> +        return super.nextLong();
>>>>>>>>>>>> +    }
>>>>>>>>>>>> +
>>>>>>>>>>>> /** {@inheritDoc} */
>>>>>>>>>>>> @Override
>>>>>>>>>>>> public String toString() {
>>>>>>>>>>>>
>>>>>
>>>>>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]