On Sun, Jul 12, 2015 at 8:16 PM, Gilles <gil...@harfang.homelinux.org> wrote: > On Sun, 12 Jul 2015 19:38:45 +0200, Thomas Neidhart wrote: >> >> On 07/12/2015 04:58 PM, Phil Steitz wrote: >>> >>> On 7/12/15 2:50 AM, Thomas Neidhart wrote: >>>> >>>> On 07/11/2015 09:43 PM, Phil Steitz wrote: >>>>> >>>>> On 7/11/15 12:29 PM, Thomas Neidhart wrote: >>>>>> >>>>>> On 07/11/2015 09:08 PM, Phil Steitz wrote: >>>>>>> >>>>>>> The code implemented in MATH-1242 to improve performance of KS >>>>>>> monteCarloP in-lines efficient generation of random boolean arrays. >>>>>>> Unfortunately, I think the implementation is not quite random (see >>>>>>> comments on the ticket). To verify it, we need to be able to test >>>>>>> the random boolean array generation directly. To do that, we have >>>>>>> to either expose the method (at least as protected) in the KS class >>>>>>> or add it somewhere else. I propose the latter but am not sure >>>>>>> where to put it. For speed, we need to avoid array copies, so the >>>>>>> API will have to be something like randomize(boolean[], nTrue). It >>>>>>> could go in the swelling MathArrays class, or RandomDataGenerator. >>>>>>> The latter probably makes more sense, but the API does not fit too >>>>>>> well. Any ideas? >>>>>> >>>>>> If it is just for testing purposes, you can also access the method in >>>>>> question via reflection, see an example here: >>>>>> >>>>>> >>>>>> http://stackoverflow.com/questions/34571/how-to-test-a-class-that-has-private-methods-fields-or-inner-classes >>>>> >>>>> Do you think it *should* be a private method of the K-S class? >>>> >>>> Right now, I do not see much uses outside the class, but if we decide to >>>> make it public then I would prefer a special util class in the random >>>> package to avoid cluttering the MathArrays class. >>> >>> >>> OK, for now we can make it private and use the reflection method >>> above to test it. >> >> >> ok, but I guess it is also fine to make it package private as sebb >> suggested. We did something similar recently for some of the improved >> sampling methods provided by Otmar. >> >>>> Regarding the RandomDataGenerator: I think this class should be >>>> deprecated and replaced by a Sampler interface as proposed by Gilles. >>> >>> >>> Please consider keeping this class. Consider this a user request. >>> I have quite a few applications that use this class for two reasons: >> >> >> ok, the reason why I thought the class should be deprecated is because >> it was not kept up-to-date with all the new discrete and continuous >> distributions that we added in the last 2-3 years. If you think it is >> useful, then we can keep it of course. >> >>> 1. One object instance tied to one PRNG that generates data from >>> multiple different distributions. This is convenient. Sure, I >>> could refactor all of these apps to instantiate new objects for each >>> type of generated data and hopefully still be able to peg them to >>> one PRNG; but that is needless work that also complicates the code. >>> >>> 2. There are quite a few methods in this class that have nothing to >>> do with sampling (nextPermutation, nextHexString, nextSecureXxx, >>> etc) but which conveniently share the RandomGenerator. I guess the >>> utility methods get moved out somewhere else. Again, I end up >>> having to refactor all of my code that uses them and when I want >>> simulations to be based on a single PRNG, I have to find a way to >>> pass the RandomGenerator around to them. >>> >>> I don't yet see the need to refactor the sampling support in the >>> distributions package; but as my own apps are not impacted by this, >>> if everyone else sees the user impact of the refactoring as >>> outweighed by the benefit, I won't stand in the way. Please lets >>> just keep the RandomDataGenerator convenience class in the random >>> package in any case. I will take care of whatever adjustments are >>> needed to adapt to whatever we settle on for sampling in the >>> distributions package. >> >> >> Well, it is not really necessary to do everything together and refactor >> the distributions. >> >> Probably it is better to start the other way round, and describe what I >> want to add, and see how other things fit in: >> >> * I want a generic Sampler interface, i.e. something like this: >> ** nextSample() >> ** nextSamples(int size) >> ** nextSamples(double[] samples) > > > +1 > >> there could be a DiscreteSampler and ContinuousSampler interface to >> handle the cases for int / double values. > > > Perhaps the name should be IntegerSampler and DoubleSampler, to accomodate > future needs (LongSampler, BooleanSampler (?)).
I would prefer consistent names for distributions and corresponding samplers. The support of distributions must be of the same data type as the return values of the corresponding sampler. Therefore, I would call the samplers for RealDistribution and IntegerDistribution RealSampler and IntegerSampler, respectively. > >> The distributions could either be changed to return such a sampler as >> Gilles proposed (with the advantage that no random instance is tied to >> the distribution itself), or implement the interface directly (with the >> advantage that we would not need to refactor too much). > > > Unless I'm missing something, the refactoring would be fairly the same: > The latter case needs implementing 3 methods (2 new ones, one with a name > change). > The former needs implementing the factory method proposed in MATH-1158, > plus the same methods as above (wrapped in the object returned by the > factory method). > > > Gilles > > > >>>> One can then create a sampler for any distribution or from other >>>> sources, e.g. when needing a fast and efficient sampler without >>>> replacement (see MATH-1239). >>> >>> >>> +1 for sequential sampling. I don't follow exactly why that >>> requires refactoring the distributions; but if it helps in a way I >>> don't yet understand, that will help convince me that refactoring >>> sampling in the distributions package is worth the user pain. >> >> >> as I said above, I wanted to combine two things in one step, maybe it is >> better to go step by step. >> >> Thomas > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > Otmar --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
