You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "suyx1999 (via GitHub)" <gi...@apache.org> on 2026/03/27 07:27:50 UTC
[PR] Fix issues in Percentile and Quantile UDFs [iotdb]
suyx1999 opened a new pull request, #17375:
URL: https://github.com/apache/iotdb/pull/17375
## Fix issues in Percentile and Quantile UDFs
### Percentile
- Fix out-of-bounds index issue
- Add according unit tests
### Quantile
- Fix incorrect type conversion
- Add according unit tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update 3 UDFs: Percentile, Quantile and Cluster [iotdb]
Posted by "suyx1999 (via GitHub)" <gi...@apache.org>.
suyx1999 commented on code in PR #17375:
URL: https://github.com/apache/iotdb/pull/17375#discussion_r3043490793
##########
library-udf/src/main/java/org/apache/iotdb/library/dprofile/UDAFQuantile.java:
##########
@@ -86,38 +86,26 @@ public void terminate(PointCollector collector) throws Exception {
case DOUBLE:
collector.putDouble(0, res);
break;
- case TIMESTAMP:
- case DATE:
- case TEXT:
- case STRING:
- case BLOB:
- case BOOLEAN:
default:
break;
Review Comment:
Have fixed this issue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update 3 UDFs: Percentile, Quantile and Cluster [iotdb]
Posted by "JackieTien97 (via GitHub)" <gi...@apache.org>.
JackieTien97 merged PR #17375:
URL: https://github.com/apache/iotdb/pull/17375
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update 3 UDFs: Percentile, Quantile and Cluster [iotdb]
Posted by "suyx1999 (via GitHub)" <gi...@apache.org>.
suyx1999 commented on code in PR #17375:
URL: https://github.com/apache/iotdb/pull/17375#discussion_r3043493850
##########
library-udf/src/main/java/org/apache/iotdb/library/dlearn/util/cluster/MedoidShape.java:
##########
@@ -0,0 +1,209 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.library.dlearn.util.cluster;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Random;
+import java.util.Set;
+
+/**
+ * Coarse clustering: {@link KMeans} uses {@code min(2k, n)} clusters (n = number of windows); greedy
+ * fastKShape picks k representatives; both labels and the objective use {@link ClusterUtils#maxNcc}.
+ */
+public class MedoidShape {
+
+ private double sampleRate = 0.3;
+ private Random random = new Random();
+
+ /** Overrides the RNG used for greedy sampling (default is {@link Random#Random()}). */
+ public void setRandom(Random random) {
+ this.random = Objects.requireNonNull(random);
+ }
+
+ private double[][] centroids;
+ private int[] labels;
+
+ public void setSampleRate(double sampleRate) {
+ if (sampleRate <= 0 || sampleRate > 1.0) {
+ throw new IllegalArgumentException("sampleRate must be in (0, 1].");
+ }
+ this.sampleRate = sampleRate;
+ }
+
+ public double getSampleRate() {
+ return sampleRate;
+ }
+
+ public void fit(double[][] samples, int k, boolean normalize, int maxIterations) {
+ validate(samples, k, maxIterations);
+ int n = samples.length;
+ int dim = samples[0].length;
+
+ int coarseK = Math.min(2 * k, n);
+
+ double[][] x = new double[n][dim];
+ for (int i = 0; i < n; i++) {
+ x[i] = ClusterUtils.maybeZNormalize(samples[i], normalize);
+ }
+
+ KMeans coarse = new KMeans();
+ coarse.fit(x, coarseK, false, maxIterations);
+ double[][] euclideanCentroids = coarse.getCentroids();
+ int[] kmLabels = coarse.getLabels();
+ long[] clusterSize = new long[coarseK];
+ for (int lb : kmLabels) {
+ clusterSize[lb]++;
+ }
+
+ centroids = fastKShape(x, k, sampleRate, dim, euclideanCentroids, clusterSize, random);
+
+ labels = new int[n];
+ for (int i = 0; i < n; i++) {
+ double maxNcc = Double.NEGATIVE_INFINITY;
+ int label = -1;
+ for (int j = 0; j < k; j++) {
+ double cur = ClusterUtils.maxNcc(x[i], centroids[j]);
+ if (cur > maxNcc) {
+ maxNcc = cur;
+ label = j;
+ }
+ }
+ labels[i] = label;
+ }
+ }
+
+ public double[][] getCentroids() {
+ return centroids;
+ }
+
+ public int[] getLabels() {
+ return labels;
+ }
+
+ private static double[][] fastKShape(
+ double[][] x,
+ int k,
+ double r,
+ int dim,
+ double[][] euclideanCentroids,
+ long[] clusterSize,
+ Random rnd) {
+ int n = x.length;
+ if (n <= k) {
+ double[][] out = new double[k][dim];
+ for (int i = 0; i < n; i++) {
+ out[i] = Arrays.copyOf(x[i], dim);
+ }
+ for (int i = n; i < k; i++) {
+ out[i] = Arrays.copyOf(x[n - 1], dim);
+ }
+ return out;
+ }
+
+ List<double[]> picked = new ArrayList<>();
+ Set<Integer> coresetIdx = new HashSet<>();
+
+ for (int round = 0; round < k; round++) {
+ List<Integer> pool = new ArrayList<>();
+ for (int i = 0; i < n; i++) {
+ if (!coresetIdx.contains(i)) {
+ pool.add(i);
+ }
+ }
+ if (pool.isEmpty()) {
+ throw new IllegalStateException("fastKShape: empty candidate pool.");
+ }
+ int sampleCount = Math.max(1, (int) (r * n));
+ sampleCount = Math.min(sampleCount, pool.size());
+ Collections.shuffle(pool, rnd);
+ List<Integer> sampleIdx = pool.subList(0, sampleCount);
+
Review Comment:
Clustering techniques inherently involve randomness, which is unavoidable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update 3 UDFs: Percentile, Quantile and Cluster [iotdb]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #17375:
URL: https://github.com/apache/iotdb/pull/17375#discussion_r3043255652
##########
library-udf/src/main/java/org/apache/iotdb/library/dprofile/UDAFQuantile.java:
##########
@@ -86,38 +86,26 @@ public void terminate(PointCollector collector) throws Exception {
case DOUBLE:
collector.putDouble(0, res);
break;
- case TIMESTAMP:
- case DATE:
- case TEXT:
- case STRING:
- case BLOB:
- case BOOLEAN:
default:
break;
}
}
- private long dataToLong(Object data) {
- long result;
+ private long dataToLong(double res) {
switch (dataType) {
case INT32:
- return (int) data;
+ return (int) res;
case FLOAT:
- result = Float.floatToIntBits((float) data);
- return (float) data >= 0f ? result : result ^ Long.MAX_VALUE;
+ float f = (float) res;
+ long flBits = Float.floatToIntBits(f);
+ return f >= 0f ? flBits : flBits ^ Long.MAX_VALUE;
case INT64:
- return (long) data;
+ return (long) res;
case DOUBLE:
- result = Double.doubleToLongBits((double) data);
- return (double) data >= 0d ? result : result ^ Long.MAX_VALUE;
- case BLOB:
- case BOOLEAN:
- case STRING:
- case TEXT:
- case DATE:
- case TIMESTAMP:
+ long d = Double.doubleToLongBits(res);
+ return res >= 0d ? d : d ^ Long.MAX_VALUE;
Review Comment:
`dataToLong(double res)` still routes INT64 values through a `double` and then casts back to `long`. For INT64 values beyond 2^53, this loses integer precision and can change the quantile result ordering. Consider ingesting INT32/INT64 using `row.getInt(0)` / `row.getLong(0)` (or otherwise passing the original integer type into `dataToLong`) so the sketch sees exact values.
##########
library-udf/src/main/java/org/apache/iotdb/library/dlearn/UDTFCluster.java:
##########
@@ -0,0 +1,214 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.library.dlearn;
+
+import org.apache.iotdb.library.dlearn.util.cluster.KMeans;
+import org.apache.iotdb.library.dlearn.util.cluster.KShape;
+import org.apache.iotdb.library.dlearn.util.cluster.MedoidShape;
+import org.apache.iotdb.library.util.Util;
+import org.apache.iotdb.udf.api.UDTF;
+import org.apache.iotdb.udf.api.access.Row;
+import org.apache.iotdb.udf.api.collector.PointCollector;
+import org.apache.iotdb.udf.api.customizer.config.UDTFConfigurations;
+import org.apache.iotdb.udf.api.customizer.parameter.UDFParameterValidator;
+import org.apache.iotdb.udf.api.customizer.parameter.UDFParameters;
+import org.apache.iotdb.udf.api.customizer.strategy.RowByRowAccessStrategy;
+import org.apache.iotdb.udf.api.exception.UDFException;
+import org.apache.iotdb.udf.api.type.Type;
+
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * Clusters a time series by partitioning it into non-overlapping subsequences of length l. Parameters: l,
+ * k, method (default kmeans), norm, maxiter, output; medoidshape also uses sample_rate (greedy sampling
+ * ratio; use 1 when the window count is small). Requires at least k windows.
+ */
+public class UDTFCluster implements UDTF {
+
+ private static final String METHOD_KMEANS = "kmeans";
+ private static final String METHOD_KSHAPE = "kshape";
+ private static final String METHOD_MEDOIDSHAPE = "medoidshape";
+
+ private static final String OUTPUT_LABEL = "label";
+ private static final String OUTPUT_CENTROID = "centroid";
+
+ private static final int DEFAULT_MAX_ITER = 200;
+ private static final double DEFAULT_SAMPLE_RATE = 0.3;
+ private static final String DEFAULT_METHOD = METHOD_KMEANS;
+
+ private int l;
+ private int k;
+ private String method;
+ private boolean norm;
+ private int maxIter;
+ private String output;
+ private double sampleRate;
+
+ private final List<Long> timestamps = new ArrayList<>();
+ private final List<Double> values = new ArrayList<>();
+
+ @Override
+ public void validate(UDFParameterValidator validator) throws Exception {
+ validator
Review Comment:
UDTFCluster is documented (and described in the PR) as accepting a single input time series, but `validate()` does not enforce `validateInputSeriesNumber(1)`. Without this, queries that pass multiple input series may be accepted but only the first column is used, which is confusing for users.
```suggestion
validator
.validateInputSeriesNumber(1)
```
##########
library-udf/src/main/java/org/apache/iotdb/library/dlearn/util/cluster/MedoidShape.java:
##########
@@ -0,0 +1,209 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.library.dlearn.util.cluster;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Random;
+import java.util.Set;
+
+/**
+ * Coarse clustering: {@link KMeans} uses {@code min(2k, n)} clusters (n = number of windows); greedy
+ * fastKShape picks k representatives; both labels and the objective use {@link ClusterUtils#maxNcc}.
+ */
+public class MedoidShape {
+
+ private double sampleRate = 0.3;
+ private Random random = new Random();
+
+ /** Overrides the RNG used for greedy sampling (default is {@link Random#Random()}). */
+ public void setRandom(Random random) {
+ this.random = Objects.requireNonNull(random);
+ }
+
+ private double[][] centroids;
+ private int[] labels;
+
+ public void setSampleRate(double sampleRate) {
+ if (sampleRate <= 0 || sampleRate > 1.0) {
+ throw new IllegalArgumentException("sampleRate must be in (0, 1].");
+ }
+ this.sampleRate = sampleRate;
+ }
+
+ public double getSampleRate() {
+ return sampleRate;
+ }
+
+ public void fit(double[][] samples, int k, boolean normalize, int maxIterations) {
+ validate(samples, k, maxIterations);
+ int n = samples.length;
+ int dim = samples[0].length;
+
+ int coarseK = Math.min(2 * k, n);
+
+ double[][] x = new double[n][dim];
+ for (int i = 0; i < n; i++) {
+ x[i] = ClusterUtils.maybeZNormalize(samples[i], normalize);
+ }
+
+ KMeans coarse = new KMeans();
+ coarse.fit(x, coarseK, false, maxIterations);
+ double[][] euclideanCentroids = coarse.getCentroids();
+ int[] kmLabels = coarse.getLabels();
+ long[] clusterSize = new long[coarseK];
+ for (int lb : kmLabels) {
+ clusterSize[lb]++;
+ }
+
+ centroids = fastKShape(x, k, sampleRate, dim, euclideanCentroids, clusterSize, random);
+
+ labels = new int[n];
+ for (int i = 0; i < n; i++) {
+ double maxNcc = Double.NEGATIVE_INFINITY;
+ int label = -1;
+ for (int j = 0; j < k; j++) {
+ double cur = ClusterUtils.maxNcc(x[i], centroids[j]);
+ if (cur > maxNcc) {
+ maxNcc = cur;
+ label = j;
+ }
+ }
+ labels[i] = label;
+ }
+ }
+
+ public double[][] getCentroids() {
+ return centroids;
+ }
+
+ public int[] getLabels() {
+ return labels;
+ }
+
+ private static double[][] fastKShape(
+ double[][] x,
+ int k,
+ double r,
+ int dim,
+ double[][] euclideanCentroids,
+ long[] clusterSize,
+ Random rnd) {
+ int n = x.length;
+ if (n <= k) {
+ double[][] out = new double[k][dim];
+ for (int i = 0; i < n; i++) {
+ out[i] = Arrays.copyOf(x[i], dim);
+ }
+ for (int i = n; i < k; i++) {
+ out[i] = Arrays.copyOf(x[n - 1], dim);
+ }
+ return out;
+ }
+
+ List<double[]> picked = new ArrayList<>();
+ Set<Integer> coresetIdx = new HashSet<>();
+
+ for (int round = 0; round < k; round++) {
+ List<Integer> pool = new ArrayList<>();
+ for (int i = 0; i < n; i++) {
+ if (!coresetIdx.contains(i)) {
+ pool.add(i);
+ }
+ }
+ if (pool.isEmpty()) {
+ throw new IllegalStateException("fastKShape: empty candidate pool.");
+ }
+ int sampleCount = Math.max(1, (int) (r * n));
+ sampleCount = Math.min(sampleCount, pool.size());
+ Collections.shuffle(pool, rnd);
+ List<Integer> sampleIdx = pool.subList(0, sampleCount);
+
Review Comment:
MedoidShape.fastKShape() uses `Collections.shuffle(pool, rnd)` with a default `new Random()` seed, so results can vary between runs (especially when multiple candidates tie on `delta`, where the shuffled iteration order decides the winner). For a database UDF this can make repeated queries non-reproducible; consider making the default selection deterministic (e.g., fixed seed, no shuffle when sampling all candidates, or deterministic tie-breaking).
##########
library-udf/src/main/java/org/apache/iotdb/library/dprofile/UDAFQuantile.java:
##########
@@ -86,38 +86,26 @@ public void terminate(PointCollector collector) throws Exception {
case DOUBLE:
collector.putDouble(0, res);
break;
- case TIMESTAMP:
- case DATE:
- case TEXT:
- case STRING:
- case BLOB:
- case BOOLEAN:
default:
break;
Review Comment:
In terminate(), the rank-to-index conversion appears off by one: `findMinValueWithRank((long) (rank * sketch.getN()))` will pass `k == n` when `rank == 1`, but `getApproxRank(v)` counts values strictly `< v`, so the valid target ranks are typically in `[0, n-1]` (nearest-rank would use `ceil(n*rank)-1`). As-is, `rank=1` can return a value larger than the max input (and other ranks are shifted).
##########
library-udf/src/main/java/org/apache/iotdb/library/dprofile/util/ExactOrderStatistics.java:
##########
@@ -251,12 +234,18 @@ public static double getMad(LongArrayList nums) {
}
}
+ /** Discrete nearest-rank index into sorted data of length {@code n}; see class Javadoc. */
+ private static int discreteNearestRankIndex(int n, double phi) {
+ int idx = (int) Math.ceil(n * phi) - 1;
+ return Math.max(0, Math.min(n - 1, idx));
+ }
+
public static float getPercentile(FloatArrayList nums, double phi) {
if (nums.isEmpty()) {
throw new NoSuchElementException();
} else {
nums.sortThis();
- return nums.get((int) Math.ceil(nums.size() * phi));
+ return nums.get(discreteNearestRankIndex(nums.size(), phi));
}
Review Comment:
PR description says unit tests were added for the Percentile and Quantile UDF fixes, but this change set only adds cluster-related IT coverage (and no new tests exercising the updated Percentile/Quantile edge cases). If tests were intended, they appear to be missing from the PR.
##########
library-udf/src/main/java/org/apache/iotdb/library/dlearn/util/cluster/MedoidShape.java:
##########
@@ -0,0 +1,209 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.library.dlearn.util.cluster;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Objects;
+import java.util.Random;
+import java.util.Set;
+
+/**
+ * Coarse clustering: {@link KMeans} uses {@code min(2k, n)} clusters (n = number of windows); greedy
+ * fastKShape picks k representatives; both labels and the objective use {@link ClusterUtils#maxNcc}.
+ */
+public class MedoidShape {
+
+ private double sampleRate = 0.3;
+ private Random random = new Random();
+
+ /** Overrides the RNG used for greedy sampling (default is {@link Random#Random()}). */
+ public void setRandom(Random random) {
+ this.random = Objects.requireNonNull(random);
+ }
+
+ private double[][] centroids;
+ private int[] labels;
+
+ public void setSampleRate(double sampleRate) {
+ if (sampleRate <= 0 || sampleRate > 1.0) {
+ throw new IllegalArgumentException("sampleRate must be in (0, 1].");
+ }
+ this.sampleRate = sampleRate;
+ }
+
+ public double getSampleRate() {
+ return sampleRate;
+ }
+
+ public void fit(double[][] samples, int k, boolean normalize, int maxIterations) {
+ validate(samples, k, maxIterations);
+ int n = samples.length;
+ int dim = samples[0].length;
+
+ int coarseK = Math.min(2 * k, n);
+
+ double[][] x = new double[n][dim];
+ for (int i = 0; i < n; i++) {
+ x[i] = ClusterUtils.maybeZNormalize(samples[i], normalize);
+ }
+
+ KMeans coarse = new KMeans();
+ coarse.fit(x, coarseK, false, maxIterations);
+ double[][] euclideanCentroids = coarse.getCentroids();
+ int[] kmLabels = coarse.getLabels();
+ long[] clusterSize = new long[coarseK];
+ for (int lb : kmLabels) {
+ clusterSize[lb]++;
+ }
+
+ centroids = fastKShape(x, k, sampleRate, dim, euclideanCentroids, clusterSize, random);
+
+ labels = new int[n];
+ for (int i = 0; i < n; i++) {
+ double maxNcc = Double.NEGATIVE_INFINITY;
+ int label = -1;
+ for (int j = 0; j < k; j++) {
+ double cur = ClusterUtils.maxNcc(x[i], centroids[j]);
+ if (cur > maxNcc) {
+ maxNcc = cur;
+ label = j;
+ }
+ }
+ labels[i] = label;
+ }
+ }
+
+ public double[][] getCentroids() {
+ return centroids;
+ }
+
+ public int[] getLabels() {
+ return labels;
+ }
+
+ private static double[][] fastKShape(
+ double[][] x,
+ int k,
+ double r,
+ int dim,
+ double[][] euclideanCentroids,
+ long[] clusterSize,
+ Random rnd) {
+ int n = x.length;
+ if (n <= k) {
+ double[][] out = new double[k][dim];
+ for (int i = 0; i < n; i++) {
+ out[i] = Arrays.copyOf(x[i], dim);
+ }
+ for (int i = n; i < k; i++) {
+ out[i] = Arrays.copyOf(x[n - 1], dim);
+ }
+ return out;
+ }
+
+ List<double[]> picked = new ArrayList<>();
+ Set<Integer> coresetIdx = new HashSet<>();
+
+ for (int round = 0; round < k; round++) {
+ List<Integer> pool = new ArrayList<>();
+ for (int i = 0; i < n; i++) {
+ if (!coresetIdx.contains(i)) {
+ pool.add(i);
+ }
+ }
+ if (pool.isEmpty()) {
+ throw new IllegalStateException("fastKShape: empty candidate pool.");
+ }
+ int sampleCount = Math.max(1, (int) (r * n));
+ sampleCount = Math.min(sampleCount, pool.size());
+ Collections.shuffle(pool, rnd);
+ List<Integer> sampleIdx = pool.subList(0, sampleCount);
+
+ double maxDelta = Double.NEGATIVE_INFINITY;
+ double[] bestSeg = null;
+ int bestIdx = -1;
+
+ for (int idx : sampleIdx) {
+ double[] seq = x[idx];
+ picked.add(seq);
+ double delta = evaluateAim(picked, euclideanCentroids, clusterSize);
+ picked.remove(picked.size() - 1);
+ if (delta > maxDelta) {
+ maxDelta = delta;
+ bestSeg = Arrays.copyOf(seq, dim);
+ bestIdx = idx;
+ }
+ }
+
+ if (bestSeg == null) {
+ throw new IllegalStateException("fastKShape: no candidate selected.");
+ }
+ picked.add(bestSeg);
+ coresetIdx.add(bestIdx);
+ }
+
+ double[][] out = new double[k][dim];
+ for (int i = 0; i < k; i++) {
+ out[i] = picked.get(i);
+ }
+ return out;
+ }
+
+ private static double evaluateAim(
+ List<double[]> curCentroids, double[][] euclideanCentroids, long[] clusterSize) {
+ double res = 0.0;
+ for (int i = 0; i < euclideanCentroids.length; i++) {
+ double maxNcc = Double.NEGATIVE_INFINITY;
+ for (double[] cur : curCentroids) {
+ double n = ClusterUtils.maxNcc(cur, euclideanCentroids[i]);
+ if (n > maxNcc) {
+ maxNcc = n;
+ }
+ }
+ res += maxNcc * clusterSize[i];
+ }
+ return res;
+ }
+
+ private static void validate(double[][] samples, int k, int maxIterations) {
+ if (samples == null || samples.length == 0) {
+ throw new IllegalArgumentException("samples must be non-empty.");
+ }
+ if (k < 2) {
+ throw new IllegalArgumentException("k must be at least 2.");
+ }
Review Comment:
MedoidShape.validate() allows `k > samples.length`, unlike KMeans/KShape which enforce `2 <= k <= samples.length`. If `k` exceeds the number of samples, `fastKShape` returns duplicated centroids, which is a surprising/degenerate outcome. Consider validating the same constraint here to make the API consistent and fail fast on invalid inputs.
```suggestion
}
if (k > samples.length) {
throw new IllegalArgumentException("k must not exceed the number of samples.");
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update 3 UDFs: Percentile, Quantile and Cluster [iotdb]
Posted by "suyx1999 (via GitHub)" <gi...@apache.org>.
suyx1999 commented on code in PR #17375:
URL: https://github.com/apache/iotdb/pull/17375#discussion_r3043480424
##########
library-udf/src/main/java/org/apache/iotdb/library/dprofile/UDAFQuantile.java:
##########
@@ -86,38 +86,26 @@ public void terminate(PointCollector collector) throws Exception {
case DOUBLE:
collector.putDouble(0, res);
break;
- case TIMESTAMP:
- case DATE:
- case TEXT:
- case STRING:
- case BLOB:
- case BOOLEAN:
default:
break;
}
}
- private long dataToLong(Object data) {
- long result;
+ private long dataToLong(double res) {
switch (dataType) {
case INT32:
- return (int) data;
+ return (int) res;
case FLOAT:
- result = Float.floatToIntBits((float) data);
- return (float) data >= 0f ? result : result ^ Long.MAX_VALUE;
+ float f = (float) res;
+ long flBits = Float.floatToIntBits(f);
+ return f >= 0f ? flBits : flBits ^ Long.MAX_VALUE;
case INT64:
- return (long) data;
+ return (long) res;
case DOUBLE:
- result = Double.doubleToLongBits((double) data);
- return (double) data >= 0d ? result : result ^ Long.MAX_VALUE;
- case BLOB:
- case BOOLEAN:
- case STRING:
- case TEXT:
- case DATE:
- case TIMESTAMP:
+ long d = Double.doubleToLongBits(res);
+ return res >= 0d ? d : d ^ Long.MAX_VALUE;
Review Comment:
Have fixed this issue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Update 3 UDFs: Percentile, Quantile and Cluster [iotdb]
Posted by "suyx1999 (via GitHub)" <gi...@apache.org>.
suyx1999 commented on code in PR #17375:
URL: https://github.com/apache/iotdb/pull/17375#discussion_r3043485366
##########
library-udf/src/main/java/org/apache/iotdb/library/dprofile/util/ExactOrderStatistics.java:
##########
@@ -251,12 +234,18 @@ public static double getMad(LongArrayList nums) {
}
}
+ /** Discrete nearest-rank index into sorted data of length {@code n}; see class Javadoc. */
+ private static int discreteNearestRankIndex(int n, double phi) {
+ int idx = (int) Math.ceil(n * phi) - 1;
+ return Math.max(0, Math.min(n - 1, idx));
+ }
+
public static float getPercentile(FloatArrayList nums, double phi) {
if (nums.isEmpty()) {
throw new NoSuchElementException();
} else {
nums.sortThis();
- return nums.get((int) Math.ceil(nums.size() * phi));
+ return nums.get(discreteNearestRankIndex(nums.size(), phi));
}
Review Comment:
Remove tests from PR description. There are existing IT tests for Percentile and Quantile.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org