New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove N^2 uniqueness checks in interpolators #2031
Conversation
*/ | ||
public static double[] noDuplicates(double[] argument, String name) { | ||
notNull(argument, name); | ||
if (argument.length > 0 && DoubleStream.of(argument).distinct().count() != argument.length) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wouldn't have thought DoubleStream.of(argument).distinct().count()
was that fast?
modules/collect/src/main/java/com/opengamma/strata/collect/DoubleArrayMath.java
Outdated
Show resolved
Hide resolved
@@ -476,6 +562,10 @@ private static void swap(double[] keys, double[] values, int first, int second) | |||
|
|||
/** | |||
* Return the array lengths if they are the same, otherwise throws an {@code IllegalArgumentException}. | |||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would have just changed this to a //
comment
double[] yValuesSrt = new double[nDataPts]; | ||
|
||
xValuesSrt = Arrays.copyOf(xValues, nDataPts); | ||
double[] xValuesSrt = Arrays.copyOf(xValues, nDataPts); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect xValues.clone()
might be quicker in cases like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently .clone()
may add a cast (since it's implicitly overriden from the Object::clone()
method)
Arrays.copyOf()
delegates to System.arrayCopy
which is native, and doesn't involve a cast.
I'd have to benchmark to be sure.
Although https://stackoverflow.com/questions/12157300/clone-or-arrays-copyof says they are equivalent post JIT
for (int j = i + 1; j < nDataPts; ++j) { | ||
ArgChecker.isFalse(xValues[i] == xValues[j], "Data should be distinct"); | ||
} | ||
if (DoubleStream.of(xValues).distinct().count() != xValues.length) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ArgChecker
?
…bleArrayMath.java Co-Authored-By: Stephen Colebourne <stephen@opengamma.com>
8391972
to
c618d63
Compare
Not sure this is the best implementation - still seems a little boilerplatey.
Could definitely be streamlined with more ArgChecker methods, potentially ones that assert that a double array is sorted, unique, and doesn't contain NaN or infinity, but maybe that's overstepping what ArgChecker is responsible for.