MegaByteTron opened a new pull request, #2585:
URL: https://github.com/apache/systemds/pull/2585

   ## Summary
   
   Fixes both issues reported in 
[SYSTEMDS-3953](https://issues.apache.org/jira/projects/SYSTEMDS/issues/SYSTEMDS-3953):
   scalar `p` vs matrix `p` returning different answers because the
   `average` flag was not propagated from `pickValues` to `pickValue`, and
   even-`n` inputs averaging the two order statistics straddling `ceil(p·n)`
   — which matches R type 7 only at `p = 0.5` and matches nothing named in
   Hyndman & Fan (1996) elsewhere.
   
   Switches the SystemDS default to **R quantile type 7** across the kernel,
   CP, SP, and FED paths. Supersedes [PR 
#2497](https://github.com/apache/systemds/pull/2497)
   ([SYSTEMDS-3922]/[SYSTEMDS-3898]) which patched averaging on even `n` only
   for `p = 0.5`. Continues [PR 
#2565](https://github.com/apache/systemds/pull/2565)
   by @ywcb00 which added the failing-tests-as-`@Ignore` scaffolding
   (`d6aabf6867`); those tests are un-ignored here.
   
   IQM is intentionally preserved — it is a trimmed weighted mean, not an
   order-statistic pick, and does not fit the type-7 interpolation shape.
   `IQMTest` is the guardrail.
   
   R type 7 (1-indexed, `n = |x|`):
   ```
   h  = (n − 1) · p + 1
   lo = floor(h)                 // clamped to [1, n]
   hi = min(lo + 1, n)
   g  = h − lo                   // in [0, 1)
   Q  = (1 − g) · x[lo] + g · x[hi]
   ```
   Ticket example — `[0.239, 0.517, 0.890, 0.944]` at `p = 0.25, 0.5, 0.75`
   now returns `0.4475, 0.7035, 0.9035`, matching R.
   
   ### Commit 1 — [SYSTEMDS-3953] Switch quantile kernel to R quantile type 7
   
   `MatrixBlock.computeType7Rank(long n, double p)` is the new static
   helper returning `{lo, hi, g}` — one primitive every consumer picks
   against so the formula lives in one place. `pickUnweightedValue` /
   `pickWeightedValue` / `pickValues` / `median` rewired to type 7;
   `pickValue(double)` becomes the public API and the `pickValue(double,
   boolean average)` overload is deleted. `QuantilePickCPInstruction`
   drops the `matBlock.getLength() % 2 == 0` argument. `QuantilePickTest`
   expected values updated to type 7; `CompressedSortTest` drops the
   now-dead `pickValue(q, true)` averaging assertion.
   
   ### Commit 2 — [SYSTEMDS-3953] Rework Spark quantile pick to R type 7
   
   `processInstruction` becomes a clean switch: VALUEPICK / MEDIAN →
   `pickQuantileValues`, IQM → new private `computeIqm`. The pre-3953
   shape mixed both under a `getWeightedQuantileSummary` blob whose fields
   meant different things per operation. `pickQuantileValues` handles
   `N ≥ 1` quantiles uniformly; weighted branch pulls `(lo, hi, g)` triples
   from a new shared `extractWeightedTriples` helper. `Type7Rank` inner
   class and `getWeightedQuantileSummary` removed.
   
   Un-ignore `testQuartileArray{CP,SP}` (marked `FIXME: fix SYSTEMDS-3953`
   by David in `d6aabf6867`) and add `testQuantileEven{1,2,3}{CP,SP}` at
   `n = 128` — the odd-length default (`n = 1973`) can't exercise `g ≠ 0`
   interpolation because at classical `p` the rank lands on an integer.
   
   ### Commit 3 — [SYSTEMDS-3953] Switch federated quantile pick to R type 7
   
   `processRowQPick` computes per-quantile rank triples `(lo, hi, g)` up
   front, flattens to a deduplicated `int[]` of ranks, feeds those to
   `pickMultipleRanks` (renamed from `computeMultipleQuantiles` to signal
   its narrowed job), and interpolates at the caller. Decoupling ranks
   from interpolation keeps the multi-rank pipeline type-7-agnostic.
   Consequence: the `boolean average` thread across four signatures
   (`processRowQPick`, `createHistogram`, `getBucketWithIndex`,
   `getSingleQuantileResult`) is removed end-to-end. `computeIqm` mirrors
   the SP helper from commit 2 so the two paths read symmetrically.
   `refineBucket` shares the coarse-histogram refinement heuristic between
   `computeIqm` and `pickMultipleRanks`. MEDIAN in `processColumnQPick`
   dispatches through VALUEPICK with `p = 0.5`; the `ColMedian` inner UDF
   goes away.
   
   Weighted row-federated inherits the kernel's sum-of-weights extension
   (`N = Σw`) for free. Column-federated weighted stays out of scope,
   matching PR #2497.
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to