You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Alex Herbert <al...@gmail.com> on 2019/07/19 09:35:52 UTC
[rng] SharedStateSampler
This interface has been added in v1.3:
/**
* Applies to samplers that can share state between instances. Samplers can be created with a
* new source of randomness that sample from the same state.
*
* @param <R> Type of the sampler.
* @since 1.3
*/
public interface SharedStateSampler<R> {
/**
* Create a new instance of the sampler with the same underlying state using the given
* uniform random provider as the source of randomness.
*
* @param rng Generator of uniformly distributed random numbers.
* @return the sampler
*/
R withUniformRandomProvider(UniformRandomProvider rng);
}
All of the DiscreteSampler and ContinuousSampler classes have been updated to implement SharedStateSampler.
This is the equivalent of:
/**
* Sampler that generates values of type {@code int} and can create new instances to sample
* from the same state with a given source of randomness.
*
* @since 1.3
*/
public interface SharedStateDiscreteSampler
extends DiscreteSampler, SharedStateSampler<SharedStateDiscreteSampler> {
// Marker interface
}
If this combined marker interface is added to the library it would simplify a lot of the code for samplers which have internal delegates to avoid casting. With this interface they can hold a SharedStateDiscreteSampler rather than a DiscreteSampler delegate and directly use it to create new delegates.
For example PoissonSampler code which wraps a specific implementation:
/**
* @param rng Generator of uniformly distributed random numbers.
* @param source Source to copy.
*/
private PoissonSampler(UniformRandomProvider rng,
PoissonSampler source) {
super(null);
if (source.poissonSamplerDelegate instanceof SmallMeanPoissonSampler) {
poissonSamplerDelegate =
((SmallMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
} else {
poissonSamplerDelegate =
((LargeMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
}
}
Becomes:
private PoissonSampler(UniformRandomProvider rng,
PoissonSampler source) {
super(null);
poissonSamplerDelegate = source.poissonSamplerDelegate.withUniformRandomProvider(rng);
}
I propose to add:
public interface SharedStateDiscreteSampler
extends DiscreteSampler, SharedStateSampler<SharedStateDiscreteSampler> {
// Marker interface
}
public interface SharedStateContinuousSampler
extends ContinuousSampler, SharedStateSampler<SharedStateContinuousSampler> {
// Marker interface
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [rng] SharedStateSampler
Posted by Gilles Sadowski <gi...@gmail.com>.
Le ven. 19 juil. 2019 à 16:47, Alex Herbert <al...@gmail.com> a écrit :
>
> On 19/07/2019 15:09, Gilles Sadowski wrote:
> > Hi.
> >
> > Le ven. 19 juil. 2019 à 15:31, Alex Herbert <al...@gmail.com> a écrit :
> >> On 19/07/2019 14:15, Gilles Sadowski wrote:
> >>> Hello.
> >>>
> >>> Le ven. 19 juil. 2019 à 14:27, Alex Herbert <al...@gmail.com> a écrit :
> >>>> One principle reason for SharedStateDiscreteSampler
> >>>> and SharedStateContinuousSampler is to simplify the current design approach
> >>>> for samplers that use internal delegates to provide optimised algorithms.
> >>>> However this creates an inefficient outer class that is just wrapping the
> >>>> implementation.
> >>>>
> >>>> The idea was to simplify the implementation of the SharedStateSampler<R>
> >>>> interface for these Discrete/Continuous Samplers. However it has wider
> >>>> application to moving away from public constructors to factory methods. The
> >>>> SharedStateDiscreteSampler interface would allow the samplers package to
> >>>> move to a factory constructor model where samplers have no public
> >>>> constructor. This allows the factory method to choose the best
> >>>> implementation. This idea is recommended by Joshua Block in Effective Java
> >>>> (see [1]).
> >>>>
> >>>> For example:
> >>>>
> >>>> UniformRandomProvider rng = ...;
> >>>> double mean = ...;
> >>>> PoissonSampler sampler = new PoissonSampler(rng, mean);
> >>>>
> >>>> vs (with some potential names):
> >>>>
> >>>> SharedStateDiscreteSampler sampler =
> >>>> PoissonSampler.newSharedStateDiscreteSampler(rng, mean);
> >>>> SharedStateDiscreteSampler sampler = PoissonSampler.newInstance(rng, mean);
> >>>> SharedStateDiscreteSampler sampler = PoissonSampler.newSampler(rng, mean);
> >>>> SharedStateDiscreteSampler sampler = PoissonSampler.newPoissonSampler(rng,
> >>>> mean);
> >>>> SharedStateDiscreteSampler sampler = PoissonSampler.of(rng, mean);
> >>>>
> >>>> Currently there are some unreleased classes in v1.3 that use various
> >>>> construction methods with different approaches to fitting within the
> >>>> current design of returning an instance that supports DiscreteSampler and
> >>>> SharedStateSampler<R> and using specialised algorithms:
> >>>>
> >>>> AliasMethodDiscreteSampler
> >>>> MarsagliaTsangWangDiscreteSampler
> >>>> GuideTableDiscreteSampler
> >>>> GeometricSampler
> >>>>
> >>>> The PoissonSamplerCache could also benefit from this as it returns a
> >>>> DiscreteSampler even though the returned object is now also a
> >>>> SharedStateSampler<R>.
> >>>>
> >>>> I would advocate introducing these interfaces and switching to a factory
> >>>> method design for unreleased code. This has no binary compatibility issues.
> >>>>
> >>>> Current released code that would also benefit from this design are those
> >>>> with internal delegates:
> >>>>
> >>>> PoissonSampler
> >>>> AhrensDieterMarsagliaTsangGammaSampler
> >>>> LargeMeanPoissonSampler
> >>>> ChengBetaSampler (no delegate but it chooses an algorithm)
> >>>>
> >>>> An example of factory code would be
> >>>>
> >>>> double alpha;
> >>>> double theta
> >>>> SharedStateContinuousSampler sampler =
> >>>> AhrensDieterMarsagliaTsangGammaSampler.of(rng, alpha, theta);
> >>>> SharedStateContinuousSampler sampler =
> >>>> AhrensDieterMarsagliaTsangGammaSampler.newGammaSampler(rng, alpha, theta);
> >>>>
> >>>> Since the class name is so verbose in this case the 'of' method name does
> >>>> not appear out of place.
> >>>>
> >>>> Note that as the SharedStateSampler<R> interface is implemented by all
> >>>> (non-deprecated) distribution samplers a factory method could be added to
> >>>> every sampler. This would pave the way for removal of public constructors
> >>>> in a 2.0 release (whenever that happens).
> >>>>
> >>>> Overall the proposal is to:
> >>>>
> >>>> - Create SharedStateDiscreteSampler and SharedStateContinuousSampler
> >>>> - Simplify the implementation of SharedStateSampler<R>
> >>>> - Move to factory constructors for unreleased samplers
> >>>> - Add factory constructors to existing samplers to return optimised
> >>>> implementations
> >>> +1 (using "of" as the method name).
> >> OK. I will create a ticket.
> >>
> >> Note that when I did SharedStateSampler I avoided this implementation to
> >> minimise changes. However I recently revisited the change to the
> >> DiscreteUniformSampler to use a new faster sampling method for a uniform
> >> range (RNG-95). The result was an implementation with 5 internal classes
> >> to handle ranges:
> >>
> >> [n,n]
> > ?
> Edge case: It is currently possible to create a DiscreteUniformSampler
> where lower == upper as the bounds are inclusive. In this case the
> sample is fixed so I created a specialisation.
> >
> >> [n,m]
> >> [0,n]
> >>
> >> And specialisations for powers of 2. It works and is much faster than
> >> the current sampler but was not very clean. I'll update this when the
> >> new structure is in place.
> >>
> >> I am happy with the name 'of' for most samplers since the sampler name
> >> contains all the information. What method name would you recommend for a
> >> sampler that can sample from different distributions? This is the class
> >> and existing methods:
> >>
> >> MarsagliaTsangWangDiscreteSampler.createBinomialDistribution
> >> MarsagliaTsangWangDiscreteSampler.createPoissonDistribution
> >> MarsagliaTsangWangDiscreteSampler.createDiscreteDistribution
> >>
> >> Leave it using 'create' or change to:
> >>
> >> - 'new'
> >> - 'for'
> >> - 'of'
> >>
> >> In this case 'of' still seems OK.
> >>
> >> MarsagliaTsangWangDiscreteSampler.ofBinomialDistribution
> >> MarsagliaTsangWangDiscreteSampler.ofPoissonDistribution
> >> MarsagliaTsangWangDiscreteSampler.ofDiscreteDistribution
> > In my understanding of the convention, what follows "of" (suffix
> > and/or method's arguments) is the input while in this case, the
> > suffix (e.g. "BinomialDistribution") is the output.
> Input -> Output
>
> Generator+BinomialDistribution -> BinomialSampler
> Generator+PoissonDistribution -> PoissonSampler
> Generator+ProbabilityDistribution -> DiscreteSampler
>
> Given that the arguments are two-fold the suffix would get very verbose.
> So I prefer the subclass factories as below.
>
> >
> > How about:
> > ---CUT---
> > public abstract class MarsagliaTsangWangDiscreteSampler {
> > // ...
> >
> > public static class Binomial {
> > private Binomial() {}
> >
> > public static MarsagliaTsangWangDiscreteSampler
> > of(UniformRandomProvider rng,
> >
> > int trials,
> >
> > double probabilityOfSuccess) {
> > // ...
> > }
> > }
> >
> > public static class Poisson {
> > // ...
> > }
> >
> > public static class Discrete {
> > // ...
> > }
> > }
> > ---CUT---
> > ?
> >
> > [A more self-describing name may be in order for the "DiscreteDistribution"
> > created from a user-defined list of probability.]
>
> In CM4 a similar class is EnumeratedIntegerDistribution. This requires
> you specify the integer random values for each probability. Here the
> values are assumed to start at 0 and increment. Note the return type is
> proposed to be SharedStateDiscreteSampler giving you:
>
> SharedStateDiscreteSampler sampler = MarsagliaTsangWangDiscreteSampler.Binomial.of
> SharedStateDiscreteSampler sampler = MarsagliaTsangWangDiscreteSampler.Poisson.of
> SharedStateDiscreteSampler sampler = MarsagliaTsangWangDiscreteSampler.Enumerated.of
Yes, "Enumerated" of course...
>
> More verbose version:
>
> SharedStateDiscreteSampler sampler = MarsagliaTsangWangDiscreteSampler.BinomialDistribution.of
> SharedStateDiscreteSampler sampler = MarsagliaTsangWangDiscreteSampler.PoissonDistribution.of
> SharedStateDiscreteSampler sampler = MarsagliaTsangWangDiscreteSampler.EnumeratedDistribution.of
Given the context, I think that it'll do without "Distribution". :-)
Best,
Gilles
>
> I shall have a think while doing the conversion.
>
> >
> > Gilles
> >
> >> Perhaps I'll just make all the changes and the names can be assessed
> >> when the PR is ready. There may be others that I have missed that will
> >> turn up when changing the code.
> >>
> >> Alex
> >>
> >>
> >>> Gilles
> >>>
> >>>> Thoughts on this are welcomed.
> >>>>
> >>>> Alex
> >>>>
> >>>> [1] https://www.baeldung.com/java-constructors-vs-static-factory-methods
> >>>>
> >>>> On Fri, 19 Jul 2019 at 10:35, Alex Herbert <al...@gmail.com> wrote:
> >>>>
> >>>>> This interface has been added in v1.3:
> >>>>>
> >>>>> /**
> >>>>> * Applies to samplers that can share state between instances. Samplers
> >>>>> can be created with a
> >>>>> * new source of randomness that sample from the same state.
> >>>>> *
> >>>>> * @param <R> Type of the sampler.
> >>>>> * @since 1.3
> >>>>> */
> >>>>> public interface SharedStateSampler<R> {
> >>>>> /**
> >>>>> * Create a new instance of the sampler with the same underlying state
> >>>>> using the given
> >>>>> * uniform random provider as the source of randomness.
> >>>>> *
> >>>>> * @param rng Generator of uniformly distributed random numbers.
> >>>>> * @return the sampler
> >>>>> */
> >>>>> R withUniformRandomProvider(UniformRandomProvider rng);
> >>>>> }
> >>>>>
> >>>>> All of the DiscreteSampler and ContinuousSampler classes have been updated
> >>>>> to implement SharedStateSampler.
> >>>>>
> >>>>> This is the equivalent of:
> >>>>>
> >>>>> /**
> >>>>> * Sampler that generates values of type {@code int} and can create new
> >>>>> instances to sample
> >>>>> * from the same state with a given source of randomness.
> >>>>> *
> >>>>> * @since 1.3
> >>>>> */
> >>>>> public interface SharedStateDiscreteSampler
> >>>>> extends DiscreteSampler,
> >>>>> SharedStateSampler<SharedStateDiscreteSampler> {
> >>>>> // Marker interface
> >>>>> }
> >>>>>
> >>>>>
> >>>>> If this combined marker interface is added to the library it would
> >>>>> simplify a lot of the code for samplers which have internal delegates to
> >>>>> avoid casting. With this interface they can hold a
> >>>>> SharedStateDiscreteSampler rather than a DiscreteSampler delegate and
> >>>>> directly use it to create new delegates.
> >>>>>
> >>>>> For example PoissonSampler code which wraps a specific implementation:
> >>>>>
> >>>>> /**
> >>>>> * @param rng Generator of uniformly distributed random numbers.
> >>>>> * @param source Source to copy.
> >>>>> */
> >>>>> private PoissonSampler(UniformRandomProvider rng,
> >>>>> PoissonSampler source) {
> >>>>> super(null);
> >>>>>
> >>>>> if (source.poissonSamplerDelegate instanceof SmallMeanPoissonSampler) {
> >>>>> poissonSamplerDelegate =
> >>>>>
> >>>>> ((SmallMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
> >>>>> } else {
> >>>>> poissonSamplerDelegate =
> >>>>>
> >>>>> ((LargeMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
> >>>>> }
> >>>>> }
> >>>>>
> >>>>> Becomes:
> >>>>>
> >>>>> private PoissonSampler(UniformRandomProvider rng,
> >>>>> PoissonSampler source) {
> >>>>> super(null);
> >>>>> poissonSamplerDelegate =
> >>>>> source.poissonSamplerDelegate.withUniformRandomProvider(rng);
> >>>>> }
> >>>>>
> >>>>>
> >>>>> I propose to add:
> >>>>>
> >>>>> public interface SharedStateDiscreteSampler
> >>>>> extends DiscreteSampler,
> >>>>> SharedStateSampler<SharedStateDiscreteSampler> {
> >>>>> // Marker interface
> >>>>> }
> >>>>>
> >>>>> public interface SharedStateContinuousSampler
> >>>>> extends ContinuousSampler,
> >>>>> SharedStateSampler<SharedStateContinuousSampler> {
> >>>>> // Marker interface
> >>>>> }
> >>>>>
> >>>>>
> >>>>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [rng] SharedStateSampler
Posted by Alex Herbert <al...@gmail.com>.
On 19/07/2019 15:09, Gilles Sadowski wrote:
> Hi.
>
> Le ven. 19 juil. 2019 à 15:31, Alex Herbert <al...@gmail.com> a écrit :
>> On 19/07/2019 14:15, Gilles Sadowski wrote:
>>> Hello.
>>>
>>> Le ven. 19 juil. 2019 à 14:27, Alex Herbert <al...@gmail.com> a écrit :
>>>> One principle reason for SharedStateDiscreteSampler
>>>> and SharedStateContinuousSampler is to simplify the current design approach
>>>> for samplers that use internal delegates to provide optimised algorithms.
>>>> However this creates an inefficient outer class that is just wrapping the
>>>> implementation.
>>>>
>>>> The idea was to simplify the implementation of the SharedStateSampler<R>
>>>> interface for these Discrete/Continuous Samplers. However it has wider
>>>> application to moving away from public constructors to factory methods. The
>>>> SharedStateDiscreteSampler interface would allow the samplers package to
>>>> move to a factory constructor model where samplers have no public
>>>> constructor. This allows the factory method to choose the best
>>>> implementation. This idea is recommended by Joshua Block in Effective Java
>>>> (see [1]).
>>>>
>>>> For example:
>>>>
>>>> UniformRandomProvider rng = ...;
>>>> double mean = ...;
>>>> PoissonSampler sampler = new PoissonSampler(rng, mean);
>>>>
>>>> vs (with some potential names):
>>>>
>>>> SharedStateDiscreteSampler sampler =
>>>> PoissonSampler.newSharedStateDiscreteSampler(rng, mean);
>>>> SharedStateDiscreteSampler sampler = PoissonSampler.newInstance(rng, mean);
>>>> SharedStateDiscreteSampler sampler = PoissonSampler.newSampler(rng, mean);
>>>> SharedStateDiscreteSampler sampler = PoissonSampler.newPoissonSampler(rng,
>>>> mean);
>>>> SharedStateDiscreteSampler sampler = PoissonSampler.of(rng, mean);
>>>>
>>>> Currently there are some unreleased classes in v1.3 that use various
>>>> construction methods with different approaches to fitting within the
>>>> current design of returning an instance that supports DiscreteSampler and
>>>> SharedStateSampler<R> and using specialised algorithms:
>>>>
>>>> AliasMethodDiscreteSampler
>>>> MarsagliaTsangWangDiscreteSampler
>>>> GuideTableDiscreteSampler
>>>> GeometricSampler
>>>>
>>>> The PoissonSamplerCache could also benefit from this as it returns a
>>>> DiscreteSampler even though the returned object is now also a
>>>> SharedStateSampler<R>.
>>>>
>>>> I would advocate introducing these interfaces and switching to a factory
>>>> method design for unreleased code. This has no binary compatibility issues.
>>>>
>>>> Current released code that would also benefit from this design are those
>>>> with internal delegates:
>>>>
>>>> PoissonSampler
>>>> AhrensDieterMarsagliaTsangGammaSampler
>>>> LargeMeanPoissonSampler
>>>> ChengBetaSampler (no delegate but it chooses an algorithm)
>>>>
>>>> An example of factory code would be
>>>>
>>>> double alpha;
>>>> double theta
>>>> SharedStateContinuousSampler sampler =
>>>> AhrensDieterMarsagliaTsangGammaSampler.of(rng, alpha, theta);
>>>> SharedStateContinuousSampler sampler =
>>>> AhrensDieterMarsagliaTsangGammaSampler.newGammaSampler(rng, alpha, theta);
>>>>
>>>> Since the class name is so verbose in this case the 'of' method name does
>>>> not appear out of place.
>>>>
>>>> Note that as the SharedStateSampler<R> interface is implemented by all
>>>> (non-deprecated) distribution samplers a factory method could be added to
>>>> every sampler. This would pave the way for removal of public constructors
>>>> in a 2.0 release (whenever that happens).
>>>>
>>>> Overall the proposal is to:
>>>>
>>>> - Create SharedStateDiscreteSampler and SharedStateContinuousSampler
>>>> - Simplify the implementation of SharedStateSampler<R>
>>>> - Move to factory constructors for unreleased samplers
>>>> - Add factory constructors to existing samplers to return optimised
>>>> implementations
>>> +1 (using "of" as the method name).
>> OK. I will create a ticket.
>>
>> Note that when I did SharedStateSampler I avoided this implementation to
>> minimise changes. However I recently revisited the change to the
>> DiscreteUniformSampler to use a new faster sampling method for a uniform
>> range (RNG-95). The result was an implementation with 5 internal classes
>> to handle ranges:
>>
>> [n,n]
> ?
Edge case: It is currently possible to create a DiscreteUniformSampler
where lower == upper as the bounds are inclusive. In this case the
sample is fixed so I created a specialisation.
>
>> [n,m]
>> [0,n]
>>
>> And specialisations for powers of 2. It works and is much faster than
>> the current sampler but was not very clean. I'll update this when the
>> new structure is in place.
>>
>> I am happy with the name 'of' for most samplers since the sampler name
>> contains all the information. What method name would you recommend for a
>> sampler that can sample from different distributions? This is the class
>> and existing methods:
>>
>> MarsagliaTsangWangDiscreteSampler.createBinomialDistribution
>> MarsagliaTsangWangDiscreteSampler.createPoissonDistribution
>> MarsagliaTsangWangDiscreteSampler.createDiscreteDistribution
>>
>> Leave it using 'create' or change to:
>>
>> - 'new'
>> - 'for'
>> - 'of'
>>
>> In this case 'of' still seems OK.
>>
>> MarsagliaTsangWangDiscreteSampler.ofBinomialDistribution
>> MarsagliaTsangWangDiscreteSampler.ofPoissonDistribution
>> MarsagliaTsangWangDiscreteSampler.ofDiscreteDistribution
> In my understanding of the convention, what follows "of" (suffix
> and/or method's arguments) is the input while in this case, the
> suffix (e.g. "BinomialDistribution") is the output.
Input -> Output
Generator+BinomialDistribution -> BinomialSampler
Generator+PoissonDistribution -> PoissonSampler
Generator+ProbabilityDistribution -> DiscreteSampler
Given that the arguments are two-fold the suffix would get very verbose.
So I prefer the subclass factories as below.
>
> How about:
> ---CUT---
> public abstract class MarsagliaTsangWangDiscreteSampler {
> // ...
>
> public static class Binomial {
> private Binomial() {}
>
> public static MarsagliaTsangWangDiscreteSampler
> of(UniformRandomProvider rng,
>
> int trials,
>
> double probabilityOfSuccess) {
> // ...
> }
> }
>
> public static class Poisson {
> // ...
> }
>
> public static class Discrete {
> // ...
> }
> }
> ---CUT---
> ?
>
> [A more self-describing name may be in order for the "DiscreteDistribution"
> created from a user-defined list of probability.]
In CM4 a similar class is EnumeratedIntegerDistribution. This requires
you specify the integer random values for each probability. Here the
values are assumed to start at 0 and increment. Note the return type is
proposed to be SharedStateDiscreteSampler giving you:
SharedStateDiscreteSampler sampler = MarsagliaTsangWangDiscreteSampler.Binomial.of
SharedStateDiscreteSampler sampler = MarsagliaTsangWangDiscreteSampler.Poisson.of
SharedStateDiscreteSampler sampler = MarsagliaTsangWangDiscreteSampler.Enumerated.of
More verbose version:
SharedStateDiscreteSampler sampler = MarsagliaTsangWangDiscreteSampler.BinomialDistribution.of
SharedStateDiscreteSampler sampler = MarsagliaTsangWangDiscreteSampler.PoissonDistribution.of
SharedStateDiscreteSampler sampler = MarsagliaTsangWangDiscreteSampler.EnumeratedDistribution.of
I shall have a think while doing the conversion.
>
> Gilles
>
>> Perhaps I'll just make all the changes and the names can be assessed
>> when the PR is ready. There may be others that I have missed that will
>> turn up when changing the code.
>>
>> Alex
>>
>>
>>> Gilles
>>>
>>>> Thoughts on this are welcomed.
>>>>
>>>> Alex
>>>>
>>>> [1] https://www.baeldung.com/java-constructors-vs-static-factory-methods
>>>>
>>>> On Fri, 19 Jul 2019 at 10:35, Alex Herbert <al...@gmail.com> wrote:
>>>>
>>>>> This interface has been added in v1.3:
>>>>>
>>>>> /**
>>>>> * Applies to samplers that can share state between instances. Samplers
>>>>> can be created with a
>>>>> * new source of randomness that sample from the same state.
>>>>> *
>>>>> * @param <R> Type of the sampler.
>>>>> * @since 1.3
>>>>> */
>>>>> public interface SharedStateSampler<R> {
>>>>> /**
>>>>> * Create a new instance of the sampler with the same underlying state
>>>>> using the given
>>>>> * uniform random provider as the source of randomness.
>>>>> *
>>>>> * @param rng Generator of uniformly distributed random numbers.
>>>>> * @return the sampler
>>>>> */
>>>>> R withUniformRandomProvider(UniformRandomProvider rng);
>>>>> }
>>>>>
>>>>> All of the DiscreteSampler and ContinuousSampler classes have been updated
>>>>> to implement SharedStateSampler.
>>>>>
>>>>> This is the equivalent of:
>>>>>
>>>>> /**
>>>>> * Sampler that generates values of type {@code int} and can create new
>>>>> instances to sample
>>>>> * from the same state with a given source of randomness.
>>>>> *
>>>>> * @since 1.3
>>>>> */
>>>>> public interface SharedStateDiscreteSampler
>>>>> extends DiscreteSampler,
>>>>> SharedStateSampler<SharedStateDiscreteSampler> {
>>>>> // Marker interface
>>>>> }
>>>>>
>>>>>
>>>>> If this combined marker interface is added to the library it would
>>>>> simplify a lot of the code for samplers which have internal delegates to
>>>>> avoid casting. With this interface they can hold a
>>>>> SharedStateDiscreteSampler rather than a DiscreteSampler delegate and
>>>>> directly use it to create new delegates.
>>>>>
>>>>> For example PoissonSampler code which wraps a specific implementation:
>>>>>
>>>>> /**
>>>>> * @param rng Generator of uniformly distributed random numbers.
>>>>> * @param source Source to copy.
>>>>> */
>>>>> private PoissonSampler(UniformRandomProvider rng,
>>>>> PoissonSampler source) {
>>>>> super(null);
>>>>>
>>>>> if (source.poissonSamplerDelegate instanceof SmallMeanPoissonSampler) {
>>>>> poissonSamplerDelegate =
>>>>>
>>>>> ((SmallMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
>>>>> } else {
>>>>> poissonSamplerDelegate =
>>>>>
>>>>> ((LargeMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
>>>>> }
>>>>> }
>>>>>
>>>>> Becomes:
>>>>>
>>>>> private PoissonSampler(UniformRandomProvider rng,
>>>>> PoissonSampler source) {
>>>>> super(null);
>>>>> poissonSamplerDelegate =
>>>>> source.poissonSamplerDelegate.withUniformRandomProvider(rng);
>>>>> }
>>>>>
>>>>>
>>>>> I propose to add:
>>>>>
>>>>> public interface SharedStateDiscreteSampler
>>>>> extends DiscreteSampler,
>>>>> SharedStateSampler<SharedStateDiscreteSampler> {
>>>>> // Marker interface
>>>>> }
>>>>>
>>>>> public interface SharedStateContinuousSampler
>>>>> extends ContinuousSampler,
>>>>> SharedStateSampler<SharedStateContinuousSampler> {
>>>>> // Marker interface
>>>>> }
>>>>>
>>>>>
>>>>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [rng] SharedStateSampler
Posted by Gilles Sadowski <gi...@gmail.com>.
Hi.
Le ven. 19 juil. 2019 à 15:31, Alex Herbert <al...@gmail.com> a écrit :
>
> On 19/07/2019 14:15, Gilles Sadowski wrote:
> > Hello.
> >
> > Le ven. 19 juil. 2019 à 14:27, Alex Herbert <al...@gmail.com> a écrit :
> >> One principle reason for SharedStateDiscreteSampler
> >> and SharedStateContinuousSampler is to simplify the current design approach
> >> for samplers that use internal delegates to provide optimised algorithms.
> >> However this creates an inefficient outer class that is just wrapping the
> >> implementation.
> >>
> >> The idea was to simplify the implementation of the SharedStateSampler<R>
> >> interface for these Discrete/Continuous Samplers. However it has wider
> >> application to moving away from public constructors to factory methods. The
> >> SharedStateDiscreteSampler interface would allow the samplers package to
> >> move to a factory constructor model where samplers have no public
> >> constructor. This allows the factory method to choose the best
> >> implementation. This idea is recommended by Joshua Block in Effective Java
> >> (see [1]).
> >>
> >> For example:
> >>
> >> UniformRandomProvider rng = ...;
> >> double mean = ...;
> >> PoissonSampler sampler = new PoissonSampler(rng, mean);
> >>
> >> vs (with some potential names):
> >>
> >> SharedStateDiscreteSampler sampler =
> >> PoissonSampler.newSharedStateDiscreteSampler(rng, mean);
> >> SharedStateDiscreteSampler sampler = PoissonSampler.newInstance(rng, mean);
> >> SharedStateDiscreteSampler sampler = PoissonSampler.newSampler(rng, mean);
> >> SharedStateDiscreteSampler sampler = PoissonSampler.newPoissonSampler(rng,
> >> mean);
> >> SharedStateDiscreteSampler sampler = PoissonSampler.of(rng, mean);
> >>
> >> Currently there are some unreleased classes in v1.3 that use various
> >> construction methods with different approaches to fitting within the
> >> current design of returning an instance that supports DiscreteSampler and
> >> SharedStateSampler<R> and using specialised algorithms:
> >>
> >> AliasMethodDiscreteSampler
> >> MarsagliaTsangWangDiscreteSampler
> >> GuideTableDiscreteSampler
> >> GeometricSampler
> >>
> >> The PoissonSamplerCache could also benefit from this as it returns a
> >> DiscreteSampler even though the returned object is now also a
> >> SharedStateSampler<R>.
> >>
> >> I would advocate introducing these interfaces and switching to a factory
> >> method design for unreleased code. This has no binary compatibility issues.
> >>
> >> Current released code that would also benefit from this design are those
> >> with internal delegates:
> >>
> >> PoissonSampler
> >> AhrensDieterMarsagliaTsangGammaSampler
> >> LargeMeanPoissonSampler
> >> ChengBetaSampler (no delegate but it chooses an algorithm)
> >>
> >> An example of factory code would be
> >>
> >> double alpha;
> >> double theta
> >> SharedStateContinuousSampler sampler =
> >> AhrensDieterMarsagliaTsangGammaSampler.of(rng, alpha, theta);
> >> SharedStateContinuousSampler sampler =
> >> AhrensDieterMarsagliaTsangGammaSampler.newGammaSampler(rng, alpha, theta);
> >>
> >> Since the class name is so verbose in this case the 'of' method name does
> >> not appear out of place.
> >>
> >> Note that as the SharedStateSampler<R> interface is implemented by all
> >> (non-deprecated) distribution samplers a factory method could be added to
> >> every sampler. This would pave the way for removal of public constructors
> >> in a 2.0 release (whenever that happens).
> >>
> >> Overall the proposal is to:
> >>
> >> - Create SharedStateDiscreteSampler and SharedStateContinuousSampler
> >> - Simplify the implementation of SharedStateSampler<R>
> >> - Move to factory constructors for unreleased samplers
> >> - Add factory constructors to existing samplers to return optimised
> >> implementations
> > +1 (using "of" as the method name).
>
> OK. I will create a ticket.
>
> Note that when I did SharedStateSampler I avoided this implementation to
> minimise changes. However I recently revisited the change to the
> DiscreteUniformSampler to use a new faster sampling method for a uniform
> range (RNG-95). The result was an implementation with 5 internal classes
> to handle ranges:
>
> [n,n]
?
> [n,m]
> [0,n]
>
> And specialisations for powers of 2. It works and is much faster than
> the current sampler but was not very clean. I'll update this when the
> new structure is in place.
>
> I am happy with the name 'of' for most samplers since the sampler name
> contains all the information. What method name would you recommend for a
> sampler that can sample from different distributions? This is the class
> and existing methods:
>
> MarsagliaTsangWangDiscreteSampler.createBinomialDistribution
> MarsagliaTsangWangDiscreteSampler.createPoissonDistribution
> MarsagliaTsangWangDiscreteSampler.createDiscreteDistribution
>
> Leave it using 'create' or change to:
>
> - 'new'
> - 'for'
> - 'of'
>
> In this case 'of' still seems OK.
>
> MarsagliaTsangWangDiscreteSampler.ofBinomialDistribution
> MarsagliaTsangWangDiscreteSampler.ofPoissonDistribution
> MarsagliaTsangWangDiscreteSampler.ofDiscreteDistribution
In my understanding of the convention, what follows "of" (suffix
and/or method's arguments) is the input while in this case, the
suffix (e.g. "BinomialDistribution") is the output.
How about:
---CUT---
public abstract class MarsagliaTsangWangDiscreteSampler {
// ...
public static class Binomial {
private Binomial() {}
public static MarsagliaTsangWangDiscreteSampler
of(UniformRandomProvider rng,
int trials,
double probabilityOfSuccess) {
// ...
}
}
public static class Poisson {
// ...
}
public static class Discrete {
// ...
}
}
---CUT---
?
[A more self-describing name may be in order for the "DiscreteDistribution"
created from a user-defined list of probability.]
Gilles
> Perhaps I'll just make all the changes and the names can be assessed
> when the PR is ready. There may be others that I have missed that will
> turn up when changing the code.
>
> Alex
>
>
> >
> > Gilles
> >
> >> Thoughts on this are welcomed.
> >>
> >> Alex
> >>
> >> [1] https://www.baeldung.com/java-constructors-vs-static-factory-methods
> >>
> >> On Fri, 19 Jul 2019 at 10:35, Alex Herbert <al...@gmail.com> wrote:
> >>
> >>> This interface has been added in v1.3:
> >>>
> >>> /**
> >>> * Applies to samplers that can share state between instances. Samplers
> >>> can be created with a
> >>> * new source of randomness that sample from the same state.
> >>> *
> >>> * @param <R> Type of the sampler.
> >>> * @since 1.3
> >>> */
> >>> public interface SharedStateSampler<R> {
> >>> /**
> >>> * Create a new instance of the sampler with the same underlying state
> >>> using the given
> >>> * uniform random provider as the source of randomness.
> >>> *
> >>> * @param rng Generator of uniformly distributed random numbers.
> >>> * @return the sampler
> >>> */
> >>> R withUniformRandomProvider(UniformRandomProvider rng);
> >>> }
> >>>
> >>> All of the DiscreteSampler and ContinuousSampler classes have been updated
> >>> to implement SharedStateSampler.
> >>>
> >>> This is the equivalent of:
> >>>
> >>> /**
> >>> * Sampler that generates values of type {@code int} and can create new
> >>> instances to sample
> >>> * from the same state with a given source of randomness.
> >>> *
> >>> * @since 1.3
> >>> */
> >>> public interface SharedStateDiscreteSampler
> >>> extends DiscreteSampler,
> >>> SharedStateSampler<SharedStateDiscreteSampler> {
> >>> // Marker interface
> >>> }
> >>>
> >>>
> >>> If this combined marker interface is added to the library it would
> >>> simplify a lot of the code for samplers which have internal delegates to
> >>> avoid casting. With this interface they can hold a
> >>> SharedStateDiscreteSampler rather than a DiscreteSampler delegate and
> >>> directly use it to create new delegates.
> >>>
> >>> For example PoissonSampler code which wraps a specific implementation:
> >>>
> >>> /**
> >>> * @param rng Generator of uniformly distributed random numbers.
> >>> * @param source Source to copy.
> >>> */
> >>> private PoissonSampler(UniformRandomProvider rng,
> >>> PoissonSampler source) {
> >>> super(null);
> >>>
> >>> if (source.poissonSamplerDelegate instanceof SmallMeanPoissonSampler) {
> >>> poissonSamplerDelegate =
> >>>
> >>> ((SmallMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
> >>> } else {
> >>> poissonSamplerDelegate =
> >>>
> >>> ((LargeMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
> >>> }
> >>> }
> >>>
> >>> Becomes:
> >>>
> >>> private PoissonSampler(UniformRandomProvider rng,
> >>> PoissonSampler source) {
> >>> super(null);
> >>> poissonSamplerDelegate =
> >>> source.poissonSamplerDelegate.withUniformRandomProvider(rng);
> >>> }
> >>>
> >>>
> >>> I propose to add:
> >>>
> >>> public interface SharedStateDiscreteSampler
> >>> extends DiscreteSampler,
> >>> SharedStateSampler<SharedStateDiscreteSampler> {
> >>> // Marker interface
> >>> }
> >>>
> >>> public interface SharedStateContinuousSampler
> >>> extends ContinuousSampler,
> >>> SharedStateSampler<SharedStateContinuousSampler> {
> >>> // Marker interface
> >>> }
> >>>
> >>>
> >>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [rng] SharedStateSampler
Posted by Alex Herbert <al...@gmail.com>.
On 19/07/2019 14:15, Gilles Sadowski wrote:
> Hello.
>
> Le ven. 19 juil. 2019 à 14:27, Alex Herbert <al...@gmail.com> a écrit :
>> One principle reason for SharedStateDiscreteSampler
>> and SharedStateContinuousSampler is to simplify the current design approach
>> for samplers that use internal delegates to provide optimised algorithms.
>> However this creates an inefficient outer class that is just wrapping the
>> implementation.
>>
>> The idea was to simplify the implementation of the SharedStateSampler<R>
>> interface for these Discrete/Continuous Samplers. However it has wider
>> application to moving away from public constructors to factory methods. The
>> SharedStateDiscreteSampler interface would allow the samplers package to
>> move to a factory constructor model where samplers have no public
>> constructor. This allows the factory method to choose the best
>> implementation. This idea is recommended by Joshua Block in Effective Java
>> (see [1]).
>>
>> For example:
>>
>> UniformRandomProvider rng = ...;
>> double mean = ...;
>> PoissonSampler sampler = new PoissonSampler(rng, mean);
>>
>> vs (with some potential names):
>>
>> SharedStateDiscreteSampler sampler =
>> PoissonSampler.newSharedStateDiscreteSampler(rng, mean);
>> SharedStateDiscreteSampler sampler = PoissonSampler.newInstance(rng, mean);
>> SharedStateDiscreteSampler sampler = PoissonSampler.newSampler(rng, mean);
>> SharedStateDiscreteSampler sampler = PoissonSampler.newPoissonSampler(rng,
>> mean);
>> SharedStateDiscreteSampler sampler = PoissonSampler.of(rng, mean);
>>
>> Currently there are some unreleased classes in v1.3 that use various
>> construction methods with different approaches to fitting within the
>> current design of returning an instance that supports DiscreteSampler and
>> SharedStateSampler<R> and using specialised algorithms:
>>
>> AliasMethodDiscreteSampler
>> MarsagliaTsangWangDiscreteSampler
>> GuideTableDiscreteSampler
>> GeometricSampler
>>
>> The PoissonSamplerCache could also benefit from this as it returns a
>> DiscreteSampler even though the returned object is now also a
>> SharedStateSampler<R>.
>>
>> I would advocate introducing these interfaces and switching to a factory
>> method design for unreleased code. This has no binary compatibility issues.
>>
>> Current released code that would also benefit from this design are those
>> with internal delegates:
>>
>> PoissonSampler
>> AhrensDieterMarsagliaTsangGammaSampler
>> LargeMeanPoissonSampler
>> ChengBetaSampler (no delegate but it chooses an algorithm)
>>
>> An example of factory code would be
>>
>> double alpha;
>> double theta
>> SharedStateContinuousSampler sampler =
>> AhrensDieterMarsagliaTsangGammaSampler.of(rng, alpha, theta);
>> SharedStateContinuousSampler sampler =
>> AhrensDieterMarsagliaTsangGammaSampler.newGammaSampler(rng, alpha, theta);
>>
>> Since the class name is so verbose in this case the 'of' method name does
>> not appear out of place.
>>
>> Note that as the SharedStateSampler<R> interface is implemented by all
>> (non-deprecated) distribution samplers a factory method could be added to
>> every sampler. This would pave the way for removal of public constructors
>> in a 2.0 release (whenever that happens).
>>
>> Overall the proposal is to:
>>
>> - Create SharedStateDiscreteSampler and SharedStateContinuousSampler
>> - Simplify the implementation of SharedStateSampler<R>
>> - Move to factory constructors for unreleased samplers
>> - Add factory constructors to existing samplers to return optimised
>> implementations
> +1 (using "of" as the method name).
OK. I will create a ticket.
Note that when I did SharedStateSampler I avoided this implementation to
minimise changes. However I recently revisited the change to the
DiscreteUniformSampler to use a new faster sampling method for a uniform
range (RNG-95). The result was an implementation with 5 internal classes
to handle ranges:
[n,n]
[n,m]
[0,n]
And specialisations for powers of 2. It works and is much faster than
the current sampler but was not very clean. I'll update this when the
new structure is in place.
I am happy with the name 'of' for most samplers since the sampler name
contains all the information. What method name would you recommend for a
sampler that can sample from different distributions? This is the class
and existing methods:
MarsagliaTsangWangDiscreteSampler.createBinomialDistribution
MarsagliaTsangWangDiscreteSampler.createPoissonDistribution
MarsagliaTsangWangDiscreteSampler.createDiscreteDistribution
Leave it using 'create' or change to:
- 'new'
- 'for'
- 'of'
In this case 'of' still seems OK.
MarsagliaTsangWangDiscreteSampler.ofBinomialDistribution
MarsagliaTsangWangDiscreteSampler.ofPoissonDistribution
MarsagliaTsangWangDiscreteSampler.ofDiscreteDistribution
Perhaps I'll just make all the changes and the names can be assessed
when the PR is ready. There may be others that I have missed that will
turn up when changing the code.
Alex
>
> Gilles
>
>> Thoughts on this are welcomed.
>>
>> Alex
>>
>> [1] https://www.baeldung.com/java-constructors-vs-static-factory-methods
>>
>> On Fri, 19 Jul 2019 at 10:35, Alex Herbert <al...@gmail.com> wrote:
>>
>>> This interface has been added in v1.3:
>>>
>>> /**
>>> * Applies to samplers that can share state between instances. Samplers
>>> can be created with a
>>> * new source of randomness that sample from the same state.
>>> *
>>> * @param <R> Type of the sampler.
>>> * @since 1.3
>>> */
>>> public interface SharedStateSampler<R> {
>>> /**
>>> * Create a new instance of the sampler with the same underlying state
>>> using the given
>>> * uniform random provider as the source of randomness.
>>> *
>>> * @param rng Generator of uniformly distributed random numbers.
>>> * @return the sampler
>>> */
>>> R withUniformRandomProvider(UniformRandomProvider rng);
>>> }
>>>
>>> All of the DiscreteSampler and ContinuousSampler classes have been updated
>>> to implement SharedStateSampler.
>>>
>>> This is the equivalent of:
>>>
>>> /**
>>> * Sampler that generates values of type {@code int} and can create new
>>> instances to sample
>>> * from the same state with a given source of randomness.
>>> *
>>> * @since 1.3
>>> */
>>> public interface SharedStateDiscreteSampler
>>> extends DiscreteSampler,
>>> SharedStateSampler<SharedStateDiscreteSampler> {
>>> // Marker interface
>>> }
>>>
>>>
>>> If this combined marker interface is added to the library it would
>>> simplify a lot of the code for samplers which have internal delegates to
>>> avoid casting. With this interface they can hold a
>>> SharedStateDiscreteSampler rather than a DiscreteSampler delegate and
>>> directly use it to create new delegates.
>>>
>>> For example PoissonSampler code which wraps a specific implementation:
>>>
>>> /**
>>> * @param rng Generator of uniformly distributed random numbers.
>>> * @param source Source to copy.
>>> */
>>> private PoissonSampler(UniformRandomProvider rng,
>>> PoissonSampler source) {
>>> super(null);
>>>
>>> if (source.poissonSamplerDelegate instanceof SmallMeanPoissonSampler) {
>>> poissonSamplerDelegate =
>>>
>>> ((SmallMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
>>> } else {
>>> poissonSamplerDelegate =
>>>
>>> ((LargeMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
>>> }
>>> }
>>>
>>> Becomes:
>>>
>>> private PoissonSampler(UniformRandomProvider rng,
>>> PoissonSampler source) {
>>> super(null);
>>> poissonSamplerDelegate =
>>> source.poissonSamplerDelegate.withUniformRandomProvider(rng);
>>> }
>>>
>>>
>>> I propose to add:
>>>
>>> public interface SharedStateDiscreteSampler
>>> extends DiscreteSampler,
>>> SharedStateSampler<SharedStateDiscreteSampler> {
>>> // Marker interface
>>> }
>>>
>>> public interface SharedStateContinuousSampler
>>> extends ContinuousSampler,
>>> SharedStateSampler<SharedStateContinuousSampler> {
>>> // Marker interface
>>> }
>>>
>>>
>>>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [rng] SharedStateSampler
Posted by Gilles Sadowski <gi...@gmail.com>.
Hello.
Le ven. 19 juil. 2019 à 14:27, Alex Herbert <al...@gmail.com> a écrit :
>
> One principle reason for SharedStateDiscreteSampler
> and SharedStateContinuousSampler is to simplify the current design approach
> for samplers that use internal delegates to provide optimised algorithms.
> However this creates an inefficient outer class that is just wrapping the
> implementation.
>
> The idea was to simplify the implementation of the SharedStateSampler<R>
> interface for these Discrete/Continuous Samplers. However it has wider
> application to moving away from public constructors to factory methods. The
> SharedStateDiscreteSampler interface would allow the samplers package to
> move to a factory constructor model where samplers have no public
> constructor. This allows the factory method to choose the best
> implementation. This idea is recommended by Joshua Block in Effective Java
> (see [1]).
>
> For example:
>
> UniformRandomProvider rng = ...;
> double mean = ...;
> PoissonSampler sampler = new PoissonSampler(rng, mean);
>
> vs (with some potential names):
>
> SharedStateDiscreteSampler sampler =
> PoissonSampler.newSharedStateDiscreteSampler(rng, mean);
> SharedStateDiscreteSampler sampler = PoissonSampler.newInstance(rng, mean);
> SharedStateDiscreteSampler sampler = PoissonSampler.newSampler(rng, mean);
> SharedStateDiscreteSampler sampler = PoissonSampler.newPoissonSampler(rng,
> mean);
> SharedStateDiscreteSampler sampler = PoissonSampler.of(rng, mean);
>
> Currently there are some unreleased classes in v1.3 that use various
> construction methods with different approaches to fitting within the
> current design of returning an instance that supports DiscreteSampler and
> SharedStateSampler<R> and using specialised algorithms:
>
> AliasMethodDiscreteSampler
> MarsagliaTsangWangDiscreteSampler
> GuideTableDiscreteSampler
> GeometricSampler
>
> The PoissonSamplerCache could also benefit from this as it returns a
> DiscreteSampler even though the returned object is now also a
> SharedStateSampler<R>.
>
> I would advocate introducing these interfaces and switching to a factory
> method design for unreleased code. This has no binary compatibility issues.
>
> Current released code that would also benefit from this design are those
> with internal delegates:
>
> PoissonSampler
> AhrensDieterMarsagliaTsangGammaSampler
> LargeMeanPoissonSampler
> ChengBetaSampler (no delegate but it chooses an algorithm)
>
> An example of factory code would be
>
> double alpha;
> double theta
> SharedStateContinuousSampler sampler =
> AhrensDieterMarsagliaTsangGammaSampler.of(rng, alpha, theta);
> SharedStateContinuousSampler sampler =
> AhrensDieterMarsagliaTsangGammaSampler.newGammaSampler(rng, alpha, theta);
>
> Since the class name is so verbose in this case the 'of' method name does
> not appear out of place.
>
> Note that as the SharedStateSampler<R> interface is implemented by all
> (non-deprecated) distribution samplers a factory method could be added to
> every sampler. This would pave the way for removal of public constructors
> in a 2.0 release (whenever that happens).
>
> Overall the proposal is to:
>
> - Create SharedStateDiscreteSampler and SharedStateContinuousSampler
> - Simplify the implementation of SharedStateSampler<R>
> - Move to factory constructors for unreleased samplers
> - Add factory constructors to existing samplers to return optimised
> implementations
+1 (using "of" as the method name).
Gilles
>
> Thoughts on this are welcomed.
>
> Alex
>
> [1] https://www.baeldung.com/java-constructors-vs-static-factory-methods
>
> On Fri, 19 Jul 2019 at 10:35, Alex Herbert <al...@gmail.com> wrote:
>
> > This interface has been added in v1.3:
> >
> > /**
> > * Applies to samplers that can share state between instances. Samplers
> > can be created with a
> > * new source of randomness that sample from the same state.
> > *
> > * @param <R> Type of the sampler.
> > * @since 1.3
> > */
> > public interface SharedStateSampler<R> {
> > /**
> > * Create a new instance of the sampler with the same underlying state
> > using the given
> > * uniform random provider as the source of randomness.
> > *
> > * @param rng Generator of uniformly distributed random numbers.
> > * @return the sampler
> > */
> > R withUniformRandomProvider(UniformRandomProvider rng);
> > }
> >
> > All of the DiscreteSampler and ContinuousSampler classes have been updated
> > to implement SharedStateSampler.
> >
> > This is the equivalent of:
> >
> > /**
> > * Sampler that generates values of type {@code int} and can create new
> > instances to sample
> > * from the same state with a given source of randomness.
> > *
> > * @since 1.3
> > */
> > public interface SharedStateDiscreteSampler
> > extends DiscreteSampler,
> > SharedStateSampler<SharedStateDiscreteSampler> {
> > // Marker interface
> > }
> >
> >
> > If this combined marker interface is added to the library it would
> > simplify a lot of the code for samplers which have internal delegates to
> > avoid casting. With this interface they can hold a
> > SharedStateDiscreteSampler rather than a DiscreteSampler delegate and
> > directly use it to create new delegates.
> >
> > For example PoissonSampler code which wraps a specific implementation:
> >
> > /**
> > * @param rng Generator of uniformly distributed random numbers.
> > * @param source Source to copy.
> > */
> > private PoissonSampler(UniformRandomProvider rng,
> > PoissonSampler source) {
> > super(null);
> >
> > if (source.poissonSamplerDelegate instanceof SmallMeanPoissonSampler) {
> > poissonSamplerDelegate =
> >
> > ((SmallMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
> > } else {
> > poissonSamplerDelegate =
> >
> > ((LargeMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
> > }
> > }
> >
> > Becomes:
> >
> > private PoissonSampler(UniformRandomProvider rng,
> > PoissonSampler source) {
> > super(null);
> > poissonSamplerDelegate =
> > source.poissonSamplerDelegate.withUniformRandomProvider(rng);
> > }
> >
> >
> > I propose to add:
> >
> > public interface SharedStateDiscreteSampler
> > extends DiscreteSampler,
> > SharedStateSampler<SharedStateDiscreteSampler> {
> > // Marker interface
> > }
> >
> > public interface SharedStateContinuousSampler
> > extends ContinuousSampler,
> > SharedStateSampler<SharedStateContinuousSampler> {
> > // Marker interface
> > }
> >
> >
> >
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: [rng] SharedStateSampler
Posted by Alex Herbert <al...@gmail.com>.
One principle reason for SharedStateDiscreteSampler
and SharedStateContinuousSampler is to simplify the current design approach
for samplers that use internal delegates to provide optimised algorithms.
However this creates an inefficient outer class that is just wrapping the
implementation.
The idea was to simplify the implementation of the SharedStateSampler<R>
interface for these Discrete/Continuous Samplers. However it has wider
application to moving away from public constructors to factory methods. The
SharedStateDiscreteSampler interface would allow the samplers package to
move to a factory constructor model where samplers have no public
constructor. This allows the factory method to choose the best
implementation. This idea is recommended by Joshua Block in Effective Java
(see [1]).
For example:
UniformRandomProvider rng = ...;
double mean = ...;
PoissonSampler sampler = new PoissonSampler(rng, mean);
vs (with some potential names):
SharedStateDiscreteSampler sampler =
PoissonSampler.newSharedStateDiscreteSampler(rng, mean);
SharedStateDiscreteSampler sampler = PoissonSampler.newInstance(rng, mean);
SharedStateDiscreteSampler sampler = PoissonSampler.newSampler(rng, mean);
SharedStateDiscreteSampler sampler = PoissonSampler.newPoissonSampler(rng,
mean);
SharedStateDiscreteSampler sampler = PoissonSampler.of(rng, mean);
Currently there are some unreleased classes in v1.3 that use various
construction methods with different approaches to fitting within the
current design of returning an instance that supports DiscreteSampler and
SharedStateSampler<R> and using specialised algorithms:
AliasMethodDiscreteSampler
MarsagliaTsangWangDiscreteSampler
GuideTableDiscreteSampler
GeometricSampler
The PoissonSamplerCache could also benefit from this as it returns a
DiscreteSampler even though the returned object is now also a
SharedStateSampler<R>.
I would advocate introducing these interfaces and switching to a factory
method design for unreleased code. This has no binary compatibility issues.
Current released code that would also benefit from this design are those
with internal delegates:
PoissonSampler
AhrensDieterMarsagliaTsangGammaSampler
LargeMeanPoissonSampler
ChengBetaSampler (no delegate but it chooses an algorithm)
An example of factory code would be
double alpha;
double theta
SharedStateContinuousSampler sampler =
AhrensDieterMarsagliaTsangGammaSampler.of(rng, alpha, theta);
SharedStateContinuousSampler sampler =
AhrensDieterMarsagliaTsangGammaSampler.newGammaSampler(rng, alpha, theta);
Since the class name is so verbose in this case the 'of' method name does
not appear out of place.
Note that as the SharedStateSampler<R> interface is implemented by all
(non-deprecated) distribution samplers a factory method could be added to
every sampler. This would pave the way for removal of public constructors
in a 2.0 release (whenever that happens).
Overall the proposal is to:
- Create SharedStateDiscreteSampler and SharedStateContinuousSampler
- Simplify the implementation of SharedStateSampler<R>
- Move to factory constructors for unreleased samplers
- Add factory constructors to existing samplers to return optimised
implementations
Thoughts on this are welcomed.
Alex
[1] https://www.baeldung.com/java-constructors-vs-static-factory-methods
On Fri, 19 Jul 2019 at 10:35, Alex Herbert <al...@gmail.com> wrote:
> This interface has been added in v1.3:
>
> /**
> * Applies to samplers that can share state between instances. Samplers
> can be created with a
> * new source of randomness that sample from the same state.
> *
> * @param <R> Type of the sampler.
> * @since 1.3
> */
> public interface SharedStateSampler<R> {
> /**
> * Create a new instance of the sampler with the same underlying state
> using the given
> * uniform random provider as the source of randomness.
> *
> * @param rng Generator of uniformly distributed random numbers.
> * @return the sampler
> */
> R withUniformRandomProvider(UniformRandomProvider rng);
> }
>
> All of the DiscreteSampler and ContinuousSampler classes have been updated
> to implement SharedStateSampler.
>
> This is the equivalent of:
>
> /**
> * Sampler that generates values of type {@code int} and can create new
> instances to sample
> * from the same state with a given source of randomness.
> *
> * @since 1.3
> */
> public interface SharedStateDiscreteSampler
> extends DiscreteSampler,
> SharedStateSampler<SharedStateDiscreteSampler> {
> // Marker interface
> }
>
>
> If this combined marker interface is added to the library it would
> simplify a lot of the code for samplers which have internal delegates to
> avoid casting. With this interface they can hold a
> SharedStateDiscreteSampler rather than a DiscreteSampler delegate and
> directly use it to create new delegates.
>
> For example PoissonSampler code which wraps a specific implementation:
>
> /**
> * @param rng Generator of uniformly distributed random numbers.
> * @param source Source to copy.
> */
> private PoissonSampler(UniformRandomProvider rng,
> PoissonSampler source) {
> super(null);
>
> if (source.poissonSamplerDelegate instanceof SmallMeanPoissonSampler) {
> poissonSamplerDelegate =
>
> ((SmallMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
> } else {
> poissonSamplerDelegate =
>
> ((LargeMeanPoissonSampler)source.poissonSamplerDelegate).withUniformRandomProvider(rng);
> }
> }
>
> Becomes:
>
> private PoissonSampler(UniformRandomProvider rng,
> PoissonSampler source) {
> super(null);
> poissonSamplerDelegate =
> source.poissonSamplerDelegate.withUniformRandomProvider(rng);
> }
>
>
> I propose to add:
>
> public interface SharedStateDiscreteSampler
> extends DiscreteSampler,
> SharedStateSampler<SharedStateDiscreteSampler> {
> // Marker interface
> }
>
> public interface SharedStateContinuousSampler
> extends ContinuousSampler,
> SharedStateSampler<SharedStateContinuousSampler> {
> // Marker interface
> }
>
>
>