From c2d91aa8eb79392419409d8c347a3e6386374de3 Mon Sep 17 00:00:00 2001 From: MegaByteTron Date: Sun, 21 Jun 2026 21:50:40 +0200 Subject: [PATCH] [SYSTEMDS-3922] Fix federated quantile VALUEPICK averaging for even-length input The federated QuantilePickFEDInstruction only enabled the even-length averaging branch for MEDIAN, while the CP reference path (QuantilePickCPInstruction) applies the same averaging convention for VALUEPICK by passing matBlock.getLength()%2==0 to MatrixBlock.pickValue. As a result, quantile(A, p) on a federated matrix with an even row count diverged from the CP reference (which compareResults(1e-9) rejects), causing the federatedQuantile{1,2,3}{CP,SP} tests to fail. They were previously marked @Ignore to hide the failure. Extend the average flag to also cover VALUEPICK so the federated path mirrors the CP averaging contract. The existing guard average = average && (... rows ...) % 2 == 0 keeps averaging disabled for odd-length input, matching CP. IQM is intentionally excluded because it has its own interpolation path. Un-ignore the six previously-disabled tests. --- .../instructions/fed/QuantilePickFEDInstruction.java | 2 +- .../federated/primitives/part2/FederatedQuantileTest.java | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/src/main/java/org/apache/sysds/runtime/instructions/fed/QuantilePickFEDInstruction.java b/src/main/java/org/apache/sysds/runtime/instructions/fed/QuantilePickFEDInstruction.java index e70f1dffa63..c5753bcac9c 100644 --- a/src/main/java/org/apache/sysds/runtime/instructions/fed/QuantilePickFEDInstruction.java +++ b/src/main/java/org/apache/sysds/runtime/instructions/fed/QuantilePickFEDInstruction.java @@ -234,7 +234,7 @@ public MatrixBlock getEquiHeightBins(ExecutionContext ec, int colID, double[ public void processRowQPick(ExecutionContext ec) { MatrixObject in = ec.getMatrixObject(input1); FederationMap fedMap = in.getFedMapping(); - boolean average = _type == OperationTypes.MEDIAN; + boolean average = _type == OperationTypes.MEDIAN || _type == OperationTypes.VALUEPICK; double[] quantiles = input2 != null ? (input2.isMatrix() ? ec.getMatrixInput(input2).getDenseBlockValues() : input2.isScalar() ? new double[] {ec.getScalarInput(input2).getDoubleValue()} : null) : diff --git a/src/test/java/org/apache/sysds/test/functions/federated/primitives/part2/FederatedQuantileTest.java b/src/test/java/org/apache/sysds/test/functions/federated/primitives/part2/FederatedQuantileTest.java index 6f13fce81b2..57ca28055a4 100644 --- a/src/test/java/org/apache/sysds/test/functions/federated/primitives/part2/FederatedQuantileTest.java +++ b/src/test/java/org/apache/sysds/test/functions/federated/primitives/part2/FederatedQuantileTest.java @@ -30,7 +30,6 @@ import org.apache.sysds.test.TestConfiguration; import org.apache.sysds.test.TestUtils; import org.junit.Assert; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -71,19 +70,16 @@ public void setUp() { } @Test - @Ignore public void federatedQuantile1CP() { federatedQuartile(Types.ExecMode.SINGLE_NODE, TEST_NAME1, 0.25); } @Test - @Ignore public void federatedQuantile2CP() { federatedQuartile(Types.ExecMode.SINGLE_NODE, TEST_NAME1, 0.5); } @Test - @Ignore public void federatedQuantile3CP() { federatedQuartile(Types.ExecMode.SINGLE_NODE, TEST_NAME1, 0.75); } @@ -104,19 +100,16 @@ public void federatedQuantilesCP() { } @Test - @Ignore public void federatedQuantile1SP() { federatedQuartile(Types.ExecMode.SPARK, TEST_NAME1, 0.25); } @Test - @Ignore public void federatedQuantile2SP() { federatedQuartile(Types.ExecMode.SPARK, TEST_NAME1, 0.5); } @Test - @Ignore public void federatedQuantile3SP() { federatedQuartile(Types.ExecMode.SPARK, TEST_NAME1, 0.75); }