You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by ru...@apache.org on 2026/09/16 14:33:04 UTC
(spark) branch branch-4.3 updated: [SPARK-59562][ML] Fix stale values in reused multinomial logistic aggregation buffer
This is an automated email from the ASF dual-hosted git repository.
zhengruifeng pushed a commit to branch branch-4.3
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.3 by this push:
new 7f6834790712 [SPARK-59562][ML] Fix stale values in reused multinomial logistic aggregation buffer
7f6834790712 is described below
commit 7f68347907120903c73e59b642786a78a2075089
Author: Ruifeng Zheng <ru...@apache.org>
AuthorDate: Wed Sep 16 22:32:03 2026 +0800
[SPARK-59562][ML] Fix stale values in reused multinomial logistic aggregation buffer
### What changes were proposed in this pull request?
This PR initializes every active class slice of the reused multinomial logistic aggregation
buffer with its margin offset before calling GEMM with `beta = 1.0`.
It also adds regression coverage that compares a single block with multiple dense and sparse
blocks. The second block is smaller, and one intercept is zero.
### Why are the changes needed?
`MultinomialLogisticBlockAggregator` reuses its margin buffer across blocks. Previously, slices
whose offset was zero were not cleared. They could therefore retain multipliers produced while
processing the previous block. GEMM then added the next block's margins to those stale values,
producing incorrect loss and gradient results.
The bug was introduced by commit
[`a24dfd697c75`](https://github.com/apache/spark/commit/a24dfd697c75e8fba776bc483b8d63103802c2cb)
(`[SPARK-39114][ML] ml.optim.aggregator avoid re-allocating buffers`), which changed the margin
array from a fresh per-block allocation to a reused buffer while retaining the zero-offset fill
skip that had only been safe for a newly allocated array.
See https://issues.apache.org/jira/browse/SPARK-59562.
### Does this PR introduce _any_ user-facing change?
Yes. Multinomial logistic regression now computes the correct loss and gradient when an
aggregator reuses its buffer across blocks and a class has a zero margin offset.
### How was this patch tested?
Added dense and sparse multi-block regression coverage and ran:
```
JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64 ./build/sbt \
'mllib/testOnly org.apache.spark.ml.optim.aggregator.MultinomialLogisticBlockAggregatorSuite'
```
All 9 tests passed.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: OpenAI Codex (GPT-5)
Closes #58848 from zhengruifeng/ml-fix-multinomial-buffer-dev-4.
Authored-by: Ruifeng Zheng <ru...@apache.org>
Signed-off-by: Ruifeng Zheng <ru...@foxmail.com>
(cherry picked from commit 2ec543b545346bfd01492572d925cf45f489878d)
Signed-off-by: Ruifeng Zheng <ru...@foxmail.com>
---
.../MultinomialLogisticBlockAggregator.scala | 2 +-
.../MultinomialLogisticBlockAggregatorSuite.scala | 27 ++++++++++++++++++++++
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/mllib/src/main/scala/org/apache/spark/ml/optim/aggregator/MultinomialLogisticBlockAggregator.scala b/mllib/src/main/scala/org/apache/spark/ml/optim/aggregator/MultinomialLogisticBlockAggregator.scala
index daf42a31ec69..26b9da022e0c 100644
--- a/mllib/src/main/scala/org/apache/spark/ml/optim/aggregator/MultinomialLogisticBlockAggregator.scala
+++ b/mllib/src/main/scala/org/apache/spark/ml/optim/aggregator/MultinomialLogisticBlockAggregator.scala
@@ -120,7 +120,7 @@ private[ml] class MultinomialLogisticBlockAggregator(
val offset = if (fitWithMean) marginOffset else intercept
var j = 0
while (j < numClasses) {
- if (offset(j) != 0) java.util.Arrays.fill(arr, j * size, (j + 1) * size, offset(j))
+ java.util.Arrays.fill(arr, j * size, (j + 1) * size, offset(j))
j += 1
}
BLAS.gemm(1.0, block.matrix, linear.transpose, 1.0, arr)
diff --git a/mllib/src/test/scala/org/apache/spark/ml/optim/aggregator/MultinomialLogisticBlockAggregatorSuite.scala b/mllib/src/test/scala/org/apache/spark/ml/optim/aggregator/MultinomialLogisticBlockAggregatorSuite.scala
index a0ffe0e7aab1..3a832726dcc2 100644
--- a/mllib/src/test/scala/org/apache/spark/ml/optim/aggregator/MultinomialLogisticBlockAggregatorSuite.scala
+++ b/mllib/src/test/scala/org/apache/spark/ml/optim/aggregator/MultinomialLogisticBlockAggregatorSuite.scala
@@ -132,6 +132,33 @@ class MultinomialLogisticBlockAggregatorSuite extends SparkFunSuite with MLlibTe
assert(aggNoIntercept.gradient.size === numFeatures * numClasses)
}
+ test("reused buffer with zero intercept") {
+ val coefficients = Vectors.dense(
+ 1.0, 2.0, -2.0, 3.0, 0.0, -1.0,
+ 4.0, 0.0, -3.0)
+ val fullBlock = InstanceBlock.fromInstances(scaledInstances.toImmutableArraySeq)
+ val expectedAggregator = getNewAggregator(instances, coefficients,
+ fitIntercept = true, fitWithMean = false)
+ expectedAggregator.add(fullBlock)
+ val expectedLoss = expectedAggregator.loss
+ val expectedGradient = expectedAggregator.gradient
+
+ val denseBlocks = Seq(
+ InstanceBlock.fromInstances(scaledInstances.take(2).toImmutableArraySeq),
+ InstanceBlock.fromInstances(scaledInstances.takeRight(1).toImmutableArraySeq))
+ val sparseBlocks = denseBlocks.map { block =>
+ new InstanceBlock(block.labels, block.weights, block.matrix.toSparseRowMajor)
+ }
+
+ Seq(denseBlocks, sparseBlocks).foreach { blocks =>
+ val aggregator = getNewAggregator(instances, coefficients,
+ fitIntercept = true, fitWithMean = false)
+ blocks.foreach(aggregator.add)
+ assert(aggregator.loss ~== expectedLoss relTol 1e-9)
+ assert(aggregator.gradient ~== expectedGradient relTol 1e-9)
+ }
+ }
+
test("check correctness: fitIntercept = false") {
val coefArray = Array(1.0, 2.0, -2.0, 3.0, 0.0, -1.0)
val numFeatures = instances.head.features.size
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org