On 11/24/15 7:28 AM, Gilles wrote: > On Tue, 24 Nov 2015 06:52:04 -0700, Phil Steitz wrote: >> I need the following methods to complete the fix for MATH-1246. I >> can add them as private methods to the KS class; but they seem >> generally useful, so I propose adding them to MathArrays. Any >> objections? >> >> /** >> * Concatenates two arrays. >> * >> * @param x first array >> * @param y second array >> * @return a new array consisting of the entries of x followed by >> the entries of y >> * @throws NullPointerException if either x or y is null >> */ >> public static double[] concat(double[] x, double[] y) > > I'd propose > public static double[] cat(double[] ... arrays)
+1 for the varArgs. I am not sure about the name though, since unix cat has a slightly different meaning. Maybe best to spell out completely "concatentate" > >> >> /** >> * Returns an array consisting of the unique values in {@code >> data}. >> * The return array is sorted in descending order. >> * >> * @param data >> * @return descending list of values included in the input array >> */ >> public static double[] values(double[] data) > > I'd suggest > public static double[] uniq(double[] data) +1, maybe spell out unique though. > >> >> /** >> * Adds random jitter to {@code data} using deviates sampled from >> {@code dist}. >> * <p> >> * Note that jitter is applied in-place - i.e., the array >> * values are overwritten with the result of applying jitter.</p> >> * >> * @param data input/output data array >> * @param dist probability distribution to sample for jitter values >> * @throws NullPointerException if either of the parameters is null >> */ >> public static void jitter(double[] data, RealDistribution dist) > > IMO, this method should be part of the new API proposed in > > https://issues.apache.org/jira/browse/MATH-1158 > > Like so: > > /** {@inheritDoc} */ > public RealDistribution.Sampler createSampler(final > RandomGenerator rng) { > return new RealDistribution.Sampler() { > public double next() { /* ... */ } > public double[] next(int sampleSize) { /* ... */ } > > /** > * @param data input/output data array. > * @param dist probability distribution to sample for > jitter values > * @throws NullPointerException if data array is null. > */ > public void jitter(double[] data) { > final int len = data.length; > final double[] jit = next(len); > for (int i = 0; i < len; i++) { > data[i] += jit[i]; > } > } > }; > } > > Advantages: > * Simpler to use (half as many arguments). :-) I guess beauty is in the eye of the beholder. Simple static array-based method looks simpler to me. > * Utility is defined where the base functionality is defined. > * Avoid adding to the overcrowded MathArrays utility class. But it is an array utility. > * Avoid dependency to another package (will help with > modularization). > > Drawbacks from usage POV > * None (?) > > Drawbacks from implementation POV > * Cannot be done before we agree to go forward with the > aforementioned issue. > > In the meantime, I suggest to implement it as a "private" method. I will add it as private. I don't like the setup above. If we do end up pulling sampling out of the distributions, I would have the method take a "sampler." > > > Regards, > Gilles > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org