[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [VOTE] Release Apache Commons RNG 1.1 based on RC6

On Tue, 7 Aug 2018 18:48:40 -0600, Gary Gregory wrote:
Hi All,

Let me rephrase the question: Is the change part of what RNG considers its
public API?

The first Clirr error was settled, as noted in the JIRA ticket.

If yes, the we must not break BC in a minor release.

1.1 should be a drop in replacement to 1.0 and not cause a runtime error.

The second error must then the fixed (by allowing the user to
shoot himself in the foot as noted below).

I apologize to Rob for wasting another RC...



On Tue, Aug 7, 2018 at 4:46 PM Gilles <gilles@xxxxxxxxxxxxxxxxxxxxx> wrote:


On Tue, 7 Aug 2018 15:11:30 -0600, Gary Gregory wrote:
> Hi All:
> Is this failure expected:
> [INFO] --- clirr-maven-plugin:2.8:check (default-cli) @
> commons-rng-sampling ---
> [INFO] Comparing to version: 1.0
> [ERROR] 5001:
> org.apache.commons.rng.sampling.distribution.BoxMullerLogNormalSampler: > Removed org.apache.commons.rng.sampling.distribution.SamplerBase from
> the
> list of superclasses
> [ERROR] 5001:
> org.apache.commons.rng.sampling.distribution.PoissonSampler:
> Removed org.apache.commons.rng.sampling.distribution.SamplerBase from
> the
> list of superclasses
> [INFO]
> ------------------------------------------------------------------------
> ?

The first, yes.[1]

The second, I overlooked.  Sorry.
The "SamplerBase" class was an easy way to not repeat boiler-plate
code and to avoid the additional indirection of composition.
The latter would have been cleaner but the choice was made amid
strong pressure (and unkind words) that the refactoring should not
loose 1% (!) of performance wrt the Commons Math implementations.[2]

The issue is only for classes that
1. inherit from "PoissonSampler",
2. call the protected methods in "SamplerBase".

I don't see any appropriate use-case but the new implementation is
not a drop-in remplacement. :-/

A fix would be to reinstate the base class and call "super(null)"
(but an application that does attempt to use the class as described
above will generate a NPE).
Calling "super(rng)" would fix the compatibility (by still allowing
"incorrect" usage).


[1] https://issues.apache.org/jira/browse/RNG-46
[2] Since then, additional RNGs were implemented that are ~40% to
     ~120% faster (depending on the type of value generated) than
     what exists in CM.

> Gary
>> [...]

To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx