This is an automated email from the ASF dual-hosted git repository.
uros-b 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 09aa4ed832ec [SPARK-58842][ML] Add descriptive messages to bare
require checks in ALS
09aa4ed832ec is described below
commit 09aa4ed832ec7c2533c896fb1463cf2a35a44447
Author: Uros <[email protected]>
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 <[email protected]>
Signed-off-by: Uros Bojanic <[email protected]>
(cherry picked from commit 12f384366cc5158f167322b56dfb5296cd93464b)
Signed-off-by: Uros Bojanic <[email protected]>
---
.../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 9a54b70f7777..fa1da6d1ba64 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
@@ -473,8 +473,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) {
@@ -888,7 +890,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.")
}
}
@@ -965,8 +968,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) {
@@ -977,7 +980,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
@@ -1327,8 +1330,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}.")
}
/**
@@ -1370,8 +1375,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}.")
}
/**
@@ -1494,8 +1501,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
@@ -1875,8 +1883,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: [email protected]
For additional commands, e-mail: [email protected]
