> On 21 Sep 2019, at 17:46, Gilles Sadowski <gillese...@gmail.com> wrote:
>
> Hello.
>
>>>>> [...]
>>>>>
>>>>>>
>>>>>> 2. Add a method BitSet<T> getSupport() to the RandomSource enum, where T
>>>>>> is an enum that can be expanded as more features are added. Initially is
>>>>>> would be:
>>>>>
>>>>> +1
>>>>> With "EnumSet", I guess.
>>>>
>>>> Yes.
>>>>
>>>>>
>>>>>>
>>>>>> enum RandomSourceSupport {
>>>>>> /** The source supported the {@link JumpableUniformRandomProvider}
>>>>>> interface. */
>>>>>> JUMPABLE,
>>>>>> /** The source supported the {@link
>>>>>> LongJumpableUniformRandomProvider}
>>>>>> interface. */
>>>>>> LONG_JUMPABLE,
>>>>>> }
>>>>>
>>>>> +1
>>>>>
>>>>>>
>>>>>> Currently the RandomSource only has a isNativeSeed(Object) method. So
>>>>>> adding methods does add API clutter. It would be possible to also add
>>>>>> getNativeSeedLength() method as this information (available in the
>>>>>> javadoc) is now available in the factory code that builds generators.
>>>>>
>>>>> +1
>>>>
>>>>
>>>> So this would add to RandomSource:
>>>>
>>>> int getSeedByteLength()
>>>> EnumSet<RandomSourceSupport> getSupport()
>>>
>>> I think that the API should not specify the type of "Set".
>>> Also, the enum should be an inner class. Thus:
>>>
>>> public class RandomSource {
>>> // ...
>>>
>>> public enum Support {
>>> JUMPABLE,
>>> // etc.
>>> }
>>>
>>> public Set<Support> getSupport() {
>>> final Set<Support> s = new EnumSet<>(Support.class);
>>>
>>> // ...
>>>
>>> return s;
>>> }
>>> }
>>
>> On reflection I think the use of Set<Enum> is not the best choice. Currently
>> we are only wanting to support the question of if this is possible:
>>
>> (JumpableUniformRandomProvider) RandomSource.create(…)
>> (LongJumpableUniformRandomProvider) RandomSource.create(…)
>>
>> The use of an enum is not a good fit here as any LongJumpable is also a
>> Jumpable due to the inheritance hierarchy. So the set of an enum that
>> specifies one or the other or both can be incorrectly used to be
>> LongJumpable but not Jumpable.
>
> I'm not sure I follow: "incorrectly used" by who?
> The library will return a consistent set (if it contains "LONG_JUMPABLE",
> it will also contain "JUMPABLE").
>
>>
>> Plus it would require returning a copy of the set for each invocation to get
>> the supported functionality.
>
> Is that a problem?
> [Assuming this is setup/one time initialization of an RNG.]
>
>> And either storing the set per enum value or computing is dynamically which
>> is possible...
>
> Yes; an implementation detail.
>
>>
>> So perhaps a method that is the inverse of the Class::isAssignableFrom
>> method. Since the RandomSourceInternal knows the RNG class this can be
>> implemented as:
>>
>> /**
>> * Determines if the class represented by this random source is either the
>> same
>> * as, or is a subclass or subinterface of, the class or interface represented
>> * by the specified {@code Class} parameter. It returns true if so; otherwise
>> it returns
>> * false.
>> *
>> * @param type the {@code Class} object to be checked
>> * @return the boolean value indicating whether the class of this random
>> source
>> * can be assigned to objects of the specified type
>> */
>> public boolean isAssignableTo(Class<?> type) {
>> return type.isAssignableFrom(internalIdentifier.getRng());
>> }
>>
>> Allowing:
>>
>> RandomSource source = ...
>> If (source.isAssignableTo(JumpableUniformRandomProvider.class) {
>> (JumpableUniformRandomProvider) RandomSource.create(source);
>> } else {
>> // fail ...
>> }
>
> Then, what have you gained over "instanceof"?
> Yes, it will avoid an instantiation; but the price seems to be an ugly API
> (IMHO).
Agreed. It is not a very nice API. A bit raw.
>
>>
>> This option is actually the easiest to implement and if more interfaces are
>> added later then nothing needs to be done since it is self-detecting.
>
> Yes, but then I'd keep that for the internal implementation of "support":
>
> public enum RandomSource {
> // ...
> public boolean isLongJumpable() {
> return isAssignableTo(LongJumpableUniformRandomProvider.class);
> }
>
> private boolean isAssignableTo(Class<?> type) {
> return type.isAssignableFrom(internalIdentifier.getRng());
> }
> }
>
> Do you foresee many other "properties" that would warrant sparing
> a trivial method for each?
The only other functionality I am aware of is:
- to advance a given number of steps (skipping). This is something that is in
the standard c++ library implementations of many generators. It is easy to do
for LCG based generators but others also. For example it is offered by the
reference PCG generators. However I do not know why you would want to do this.
- splittable. This we have been over before. It is functionality tied to the
Java 1.8 stream API and the requirement to spawn new generators for child
workers in a thread pool of unknown size (thus unknown number of descendent
children ruling out the use of jumpable). So maybe worth looking at when moving
to Java 1.8 (e.g. to mimic new Random().doubles() -> DoubleStream with any RNG
that is splittable).
I am happy to go with adding the ‘is’ properties:
isLongJumpable()
isJumpable()
The future may have:
isSkippable()
isSplittable()
Alex
>
> Gilles
>
>>>
>>>>
>>>> The first should return the native seed size in bytes. Currently the
>>>> javadoc
>>>> contains the native seed length. This is the length of the seed array or 1
>>>> if the seed is a Long/Integer.
>>>>
>>>> I think having the seed size in bytes is more generic and it doesn’t matter
>>>> if the seed is an array or not. So perhaps the javadoc should contain the
>>>> human readable array length (if relevant) but the code uses the generic
>>>> byte
>>>> length allowing:
>>>
>>> +1
>>>
>>>>
>>>> SecureRandom sr = new SecureRandom();
>>>> RandomSource source = RandomSource.MWC_256;
>>>> UniformRandomProvider rng = RandomSource.create(source,
>>>> sr.generateSeed(source.getSeedByteLength());
>>>>
>>>> In the case where the seed is a Long/Integer I suggest dropping the
>>>> javadoc:
>>>> "Native seed size: 1” as it is redundant.
>>>
>>> +1
>>>
>>> Regards,
>>> Gilles
>>>
>>>>>
>>>>>>
>>>>>> Any other suggestions?
>>>>>>
>>>>>> Alex
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
