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

Reply via email to