Add initial vector variate generation functions #1296
Conversation
| for (Double scalar : doubles) { | ||
| accumulator+=scalar*scalar; | ||
| } | ||
| double vectorLen = Math.sqrt(accumulator); |
There was a problem hiding this comment.
nit: name this vectorNorm? vector length can be potentially misinterpreted as the number of components
There was a problem hiding this comment.
I think it's too ambiguous as well. But it's not the norm. It's the scalar length that is used to compute the norm. So maybe we should call it scalarLength?
| this.funcs = new LongToDoubleFunction[dim]; | ||
| for (int i = 0; i < dim; i++) { | ||
| if (i<dims.length/2) { | ||
| funcs[i]=new HashedDoubleRange(dims[i<<1].doubleValue(),dims[i<<1].doubleValue()+1); |
There was a problem hiding this comment.
I may have glossed over this, but question: the bit shift to the left doubles index i. Should the second argument in HashedDoubleRange be:
dims[(i<<1) + 1].doubleValue()
So that we are getting for i < dim.length / 2:
i = 0 => HashedDoubleRange(dims[0], dims[1])
i = 1 => HashedDoubleRange(dims[2], dims[3])
...
? Perhaps I'm misunderstanding something.
There was a problem hiding this comment.
It looks to me like you found a bug, and either the passing test is a mathematical coincidence, or I've missed coverage here.
There was a problem hiding this comment.
Ah the test was not specific enough. I made it quantitative. The previous version was asserting only a subset of the expected range.
There was a problem hiding this comment.
👍🏼 Good catch @ShaunakDas88
| public UniformVectorSizedStepped(Number... dims) { | ||
| if (dims.length>=1 && (dims.length)%2==1 && dims[0] instanceof Integer) { | ||
| this.dim = dims[0].intValue(); | ||
| dims = Arrays.copyOfRange(dims,1,dims.length); |
There was a problem hiding this comment.
minor performance nit: Rather than making a copy from index 1 on of the array, perhaps a startOffset pointer to index 1 here (and index 0 in the else). Then the true length we care about is dims.length - startOffset, and we can just iterate through the original dims appropriately and do index arithmetic?
There was a problem hiding this comment.
I would generally agree, except that in this case the constructor logic is only used during initialzation. Once a binding function is resolved and installed, it is static, and only what happens in the apply method will be perf sensitive. Not opposed to the change, though.
There was a problem hiding this comment.
@ShaunakDas88 I had a similar question in the past, this is the preferred way for init and apply in the bindings for performance reasons.
closes #1295