Hello Samy.

Le lun. 7 juin 2021 à 11:51, Samy Badjoudj <samy.badjo...@gmail.com> a écrit :
>
> Hello Gilles,
>
> Is there something we  need to change
> https://github.com/apache/commons-math/pull/190, that one comes after
> the discussion we had.

I probably failed to convey it but the main point of that discussion
was that we need more info about the subject (and use-cases too)
in order to be confident about what the API should be (or not).

The two of us agreeing that the current design can be improved is
a necessary condition but not sufficient to ensure that e.g. your PR
is a long-term improvement:  I'd be wary to add a new package to
the "legacy" module as that would just mean that we know that
more work is needed.

I remind that a goal of the modularization is to create a non-"legacy"
module with each functionality that can stand on its own.
The low-discrepancy classes are obvious candidates.  [They depend
on the legacy exception classes but that can be trivially fixed.  The
hard part it to come up with real use-cases to drive the discussion on
the design.]

It's great if you dig further into this.

But it's also great if you take on more pragmatic issues like those
that could block the release of v4.0.  [Cf. other thread.]

Thanks,
Gilles

> Thanks
>
> On 01.06.21 20:42, Samy Badjoudj wrote:
> >
> > On 01.06.21 20:02, Gilles Sadowski wrote:
> >> Le mar. 1 juin 2021 à 15:25, Samy Badjoudj <samy.badjo...@gmail.com>
> >> a écrit :
> >>> Thanks Gilles, please find below my remarks
> >>>
> >>>
> >>> On 01.06.21 12:37, Gilles Sadowski wrote:
> >>>> Le lun. 31 mai 2021 à 02:07, Gilles Sadowski <gillese...@gmail.com>
> >>>> a écrit :
> >>>>> Hello.
> >>>>>
> >>>>> Halton and Sobol sequences have been implemented in the "random"
> >>>>> package.  From Wikipedia[1]:
> >>>>> ---CUT---
> >>>>> Low-discrepancy sequences are also called quasirandom sequences,
> >>>>> due to their common use as a replacement of uniformly distributed
> >>>>> random numbers. The "quasi" modifier is used to denote more clearly
> >>>>> that the values of a low-discrepancy sequence are neither random nor
> >>>>> pseudorandom.
> >>>>> ---CUT---
> >>>>>
> >>>>> TL;DR;
> >>>>> I propose to create an interface "LowDiscrepancySequence" to properly
> >>>>> represents the concept (as opposed to "RandomVectorGenerator").
> >>>> In current "master", interface "RandomVectorGenerator" is not used
> >>>> anymore; I'll remove it.
> >>>>
> >>>> The new (untested) API would be:
> >>>> ---CUT---
> >>>> public interface LowDiscrepancySequence extends Supplier<double[]> {
> >>>>       /**
> >>>>        * Creates a copy of the LowDiscrepancySequence and then
> >>>> advances
> >>> I would do first the advance then the copy it.
> >> Then you'd end up with two instances having the same state...
> > +1, right,  it reminds me JumpableUniformRandomProvider ;)
> >>>>   /* the state of the current instance. The copy is returned.
> >>>>        *
> >>>>        * @param jump How far to jump from the current state.
> >>>>        * @return a copy of this instance.
> >>>>        */
> >>>>       LowDiscrepancySequence jump(int jump);
> >>>> }
> >>>>
> >>>> public class SobolSequence implements LowDiscrepancySequence { /*
> >>>> ... */ }
> >>>>
> >>>> public class HaltonSequence implements LowDiscrepancySequence { /*
> >>>> ... */ }
> >>>> ---CUT---
> >>>>
> >>>> TBD (through reading about the subject): Is this interface common to
> >>>> all such algorithms?  [Is "jump" always defined?  Should its argument
> >>>> rather be a "long"?]
> >>> The maximum size of an array (underlying object) is limited by the max
> >>> integer value. I would suggest keeping an it
> >> Which "underlying" array are you referring to?
> >> How would it impact the size of the "jump"?
> > After looking at the Sobol and Halton sequences plus doing some
> > testing, we can go for a long
> >>>>   From an usage POV, I don't see the purpose of the "skipTo" and
> >>>> "getNextIndex" methods.
> >>> Agree
> >>>> By the way, Javadoc for "skipTo" mandates that the arg be positive,
> >>>> but no check is performed; and the code produces a result (expected
> >>>> or not?).
> >>> It will return 0 vector in HaltonSequenceGenerator since
> >> A bug then.
> > Right I did check this condition
> >>> for (int i =0; i <dimension; i++) {
> >>>       int index =count; double f =1.0 /base[i]; int j =0; while
> >>> (index >0) {
> >>>           final int digit = scramble(i, j, base[i], index %base[i]);
> >>> v[i] += f * digit; index /=base[i]; // floor( index / base ) f
> >>> /=base[i]; }
> >>> }
> >>>
> >>> For the SobolSequence the Gray Code bit manipulation is working just
> >>> for unsigned number so unexpected value behavior.
> >> A comparison should be done with another implementation.
> >> Maybe the range can be extended to all "int" values (?).
> >>
> >>> Meaning we can do a sanity check in the jump method before.
> >> +1
> >>
> >>>> Another improvement for "SobolGenerator" would be to move the IO
> >>>> to a factory/builder class (that would also cache the data it reads
> >>>> from
> >>>> the "resource" file).
> >>>>
> >>> That will be a good idea as a separate new improvement
> >>>
> >>>
> >>>>> [1] https://en.wikipedia.org/wiki/Low-discrepancy_sequence
> >>>>
> >>>>
> >>> /** Interface to Low Discrepancy Sequence generator and supplier *
> >>> Supplier of a low discrepancy vectors * Offers navigation through
> >>> underlying sequence */ public interface
> >>> LowDiscrepancySequenceextends Supplier<double[]> {
> >>>       /** * Skip to the index in the sequence * @param index of the
> >>> element to
> >>> skip to * @return T element at the index */
> >>> LowDiscrepancySequencejump(int index); ----> return a copy of it to
> >>> avoid side effect }
> >>>
> >>>
> >>>
> >>>
> >>> /** * private constructor avoid side effects * @return copy of
> >>> LowDiscrepancySequence */ private LowDiscrepancySequencecopy() {
> >>>       return new SobolSequenceGenerator(this); }
> >>>
> >>>
> >>> /** * private constructor avoid side effects * @return copy of
> >>> LowDiscrepancySequence */ private LowDiscrepancySequencecopy() {
> >>>       return new HaltonSequenceGenerator(this); }
> >>>
> >> Another line of improvement would be to mimic "RandomSource":
> >> https://gitbox.apache.org/repos/asf?p=commons-rng.git;a=blob;f=commons-rng-simple/src/main/java/org/apache/commons/rng/simple/RandomSource.java;h=65ca02fdc285fbb7ea8305008dbce21f571191d7;hb=HEAD
> >
> > Thanks for the link I will have a closer look, never checked the rng
> > repo btw :)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to