[
https://issues.apache.org/jira/browse/RNG-142?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17357784#comment-17357784
]
Alex Herbert commented on RNG-142:
----------------------------------
The {{NormalizedGaussianSampler}} does not implement the
{{SharedStateContinuousSampler}}. This is the issue:
{code:java}
ZigguratNormalizedGaussianSampler n01 =
ZigguratNormalizedGaussianSampler.of(RandomSource.create(RandomSource.KISS));
GaussianSampler g =
GaussianSampler.of(n01.withUniformRandomProvider(RandomSource.create(RandomSource.JSF_64)),
0.43, 2.1);
{code}
as {{ZigguratNormalizedGaussianSampler}} returns
{{SharedStateContinuousSampler}} from {{withUniformRandomProvider}}.
All the classes in the {{distribution}} package return
{{SharedStateContinuousSampler}}/{{SharedStateDiscreteSampler}} from
{{withUniformRandomProvider}}. Many could be changed to return the type of the
implementing class. However some use a delegate of a different class to do the
sampling. This is due to adaption of legacy samplers such as the PoissonSampler
and ChengBetaSampler which return samplers that do not extend from the parent
class, mainly to avoid extending the deprecated SamplerBase class. A clean
implementation would have the base class as abstract and then the
implementations can extend from this. The abstract class can specify to return
its own type and the returned class will be a sub-class. See the
AliasMethodDiscreteSampler for an example where a sub-class specialisation can
be used.
This has a problem in that both return types have advantages. It is nice to
return the same type as the class that declared to implement the interface. It
is also an advantage to be able to return any class that provides the
functionality of the sampler. This allows choosing the implementation which may
be different depending on the parameters.
The interfaces could have the return type of the method defined using a generic:
{code:java}
// Unchanged
public interface SharedStateSampler<R> {
R withUniformRandomProvider(UniformRandomProvider rng);
}
// Add <R>
public interface SharedStateContinuousSampler<R>
extends ContinuousSampler, SharedStateSampler<R> {
// Composite interface
}
// Add <R>
public interface SharedStateDiscreteSampler<R>
extends DiscreteSampler, SharedStateSampler<R> {
// Composite interface
}
// Add <R>
public interface SharedStateObjectSampler<R extends SharedStateObjectSampler<R,
T>, T> extends
ObjectSampler<T>, SharedStateSampler<R> {
// Composite interface
}
{code}
The SharedStateObjectSampler interface has not yet been released so changes
here are fine. It would change samplers like this:
{code:java}
public class CollectionSampler<T> implements SharedStateObjectSampler<T> {
// to
public class CollectionSampler<T> implements
SharedStateObjectSampler<CollectionSampler<T>, T> {
{code}
For the distributions the change requires adding a type, for example:
{code:java}
public class ZigguratNormalizedGaussianSampler
implements NormalizedGaussianSampler,
SharedStateContinuousSampler<ZigguratNormalizedGaussianSampler> {
public ZigguratNormalizedGaussianSampler
withUniformRandomProvider(UniformRandomProvider rng);
public static ZigguratNormalizedGaussianSampler of(UniformRandomProvider
rng);
}
{code}
I don't think it would break binary compatibility as the return type has
changed to a sub-class. But I would have to go through all the code to make the
changes and check. There may be some classes which are a problem.
> Return type of method "withUniformRandomProvider"
> -------------------------------------------------
>
> Key: RNG-142
> URL: https://issues.apache.org/jira/browse/RNG-142
> Project: Commons RNG
> Issue Type: Improvement
> Components: sampling
> Reporter: Gilles Sadowski
> Priority: Major
>
> Expected usage:
> {code:java}
> NormalizedGaussianSampler n01 =
> ZigguratNormalizedGaussianSampler.of(RandomSource.create(RandomSource.KISS));
> GaussianSampler g =
> GaussianSampler.of(n01.withUniformRandomProvider(RandomSource.create(RandomSource.JSF_64)),
> 0.43, 2.1);
> {code}
> Code doesn't compile: Method {{withUniformRandomProvider}} returns a
> {{SharedStateContinuousSampler}} whereas a {{NormalizedGaussianSampler}} is
> required.
> Am I missing something?
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
