You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by ur...@apache.org on 2026/08/19 13:44:38 UTC
(spark) branch branch-4.x updated: [SPARK-58842][ML] Add descriptive messages to bare require checks in ALS
This is an automated email from the ASF dual-hosted git repository.
uros-b pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new 40f81cfa65c6 [SPARK-58842][ML] Add descriptive messages to bare require checks in ALS
40f81cfa65c6 is described below
commit 40f81cfa65c6cfb3fe2322cf244f8779f48c9ec8
Author: Uros <22...@users.noreply.github.com>
AuthorDate: Wed Aug 19 13:44:14 2026 +0000
[SPARK-58842][ML] Add descriptive messages to bare require checks in ALS
### What changes were proposed in this pull request?
Adds a short descriptive message to the 14 bare `require(...)` checks in `ALS.scala`, naming the values involved:
```scala
require(c >= 0.0, s"Observation weight must be non-negative but found $c.")
require(a.length == k, s"Observation length ${a.length} must equal rank $k.")
require(blockId < numBlocks, s"blockId $blockId must be less than numBlocks $numBlocks.")
```
### Why are the changes needed?
When one of these guards trips it currently produces only `requirement failed`, which gives no indication of which invariant broke in a long-running factorization job. The file already establishes the pattern: `require(numBlocks > 0, s"numBlocks must be positive but found $numBlocks.")`. This mirrors SPARK-58621, which did the same for `SummarizerBuffer`.
### Does this PR introduce _any_ user-facing change?
No. Conditions and exception types are unchanged; only message text is added.
### How was this patch tested?
Existing `ALSSuite` coverage applies. Every condition was verified byte-identical after the change, and no test asserts on these messages: the suite's `getMessage` assertions target the unrelated `ALS only supports non-Null values` / `must be of type numeric` strings, and the `intercept[IllegalArgumentException]` blocks check only the exception type.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.8)
Closes #58075 from uros-b/error-als-require-messages.
Authored-by: Uros <22...@users.noreply.github.com>
Signed-off-by: Uros Bojanic <22...@users.noreply.github.com>
(cherry picked from commit 12f384366cc5158f167322b56dfb5296cd93464b)
Signed-off-by: Uros Bojanic <22...@users.noreply.github.com>
---
.../org/apache/spark/ml/recommendation/ALS.scala | 37 ++++++++++++++--------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala b/mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala
index 5ab6ffa046b5..b2c854b08a1c 100644
--- a/mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala
+++ b/mllib/src/main/scala/org/apache/spark/ml/recommendation/ALS.scala
@@ -460,8 +460,10 @@ class ALSModel private[ml] (
var scores: Array[Float] = null
var idxOrd: GuavaOrdering[Int] = null
iter.flatMap { case (srcIds, srcMat, dstIds, dstMat) =>
- require(srcMat.length == srcIds.length * rank)
- require(dstMat.length == dstIds.length * rank)
+ require(srcMat.length == srcIds.length * rank,
+ s"srcMat must have ${srcIds.length * rank} entries but has ${srcMat.length}.")
+ require(dstMat.length == dstIds.length * rank,
+ s"dstMat must have ${dstIds.length * rank} entries but has ${dstMat.length}.")
val m = srcIds.length
val n = dstIds.length
if (scores == null || scores.length < n) {
@@ -891,7 +893,8 @@ object ALS extends DefaultParamsReadable[ALS] with Logging {
ata = new Array[Double](rank * rank)
initialized = true
} else {
- require(this.rank == rank)
+ require(this.rank == rank,
+ s"NNLSSolver was initialized with rank ${this.rank} but got $rank.")
}
}
@@ -968,8 +971,8 @@ object ALS extends DefaultParamsReadable[ALS] with Logging {
/** Adds an observation. */
def add(a: Array[Float], b: Double, c: Double = 1.0): NormalEquation = {
- require(c >= 0.0)
- require(a.length == k)
+ require(c >= 0.0, s"Observation weight must be non-negative but found $c.")
+ require(a.length == k, s"Observation length ${a.length} must equal rank $k.")
copyToDouble(a)
BLAS.nativeBLAS.dspr(upper, k, c, da, 1, ata)
if (b != 0.0) {
@@ -980,7 +983,7 @@ object ALS extends DefaultParamsReadable[ALS] with Logging {
/** Merges another normal equation object. */
def merge(other: NormalEquation): NormalEquation = {
- require(other.k == k)
+ require(other.k == k, s"Cannot merge normal equations of rank ${other.k} into rank $k.")
BLAS.nativeBLAS.daxpy(ata.length, 1.0, other.ata, 1, ata, 1)
BLAS.nativeBLAS.daxpy(atb.length, 1.0, other.atb, 1, atb, 1)
this
@@ -1330,8 +1333,10 @@ object ALS extends DefaultParamsReadable[ALS] with Logging {
ratings: Array[Float]) {
/** Size of the block. */
def size: Int = ratings.length
- require(dstEncodedIndices.length == size)
- require(dstPtrs.length == srcIds.length + 1)
+ require(dstEncodedIndices.length == size,
+ s"dstEncodedIndices must have $size entries but has ${dstEncodedIndices.length}.")
+ require(dstPtrs.length == srcIds.length + 1,
+ s"dstPtrs must have ${srcIds.length + 1} entries but has ${dstPtrs.length}.")
}
/**
@@ -1373,8 +1378,10 @@ object ALS extends DefaultParamsReadable[ALS] with Logging {
ratings: Array[Float]) {
/** Size of the block. */
def size: Int = srcIds.length
- require(dstIds.length == srcIds.length)
- require(ratings.length == srcIds.length)
+ require(dstIds.length == srcIds.length,
+ s"dstIds must have ${srcIds.length} entries but has ${dstIds.length}.")
+ require(ratings.length == srcIds.length,
+ s"ratings must have ${srcIds.length} entries but has ${ratings.length}.")
}
/**
@@ -1497,8 +1504,9 @@ object ALS extends DefaultParamsReadable[ALS] with Logging {
dstLocalIndices: Array[Int],
ratings: Array[Float]): this.type = {
val sz = srcIds.length
- require(dstLocalIndices.length == sz)
- require(ratings.length == sz)
+ require(dstLocalIndices.length == sz,
+ s"dstLocalIndices must have $sz entries but has ${dstLocalIndices.length}.")
+ require(ratings.length == sz, s"ratings must have $sz entries but has ${ratings.length}.")
this.srcIds ++= srcIds
this.ratings ++= ratings
var j = 0
@@ -1878,8 +1886,9 @@ object ALS extends DefaultParamsReadable[ALS] with Logging {
/** Encodes a (blockId, localIndex) into a single integer. */
def encode(blockId: Int, localIndex: Int): Int = {
- require(blockId < numBlocks)
- require((localIndex & ~localIndexMask) == 0)
+ require(blockId < numBlocks, s"blockId $blockId must be less than numBlocks $numBlocks.")
+ require((localIndex & ~localIndexMask) == 0,
+ s"localIndex $localIndex must be in [0, $localIndexMask].")
(blockId << numLocalIndexBits) | localIndex
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org