Skip to content

Commit 0875765

Browse files
uros-bdongjoon-hyun
authored andcommitted
[SPARK-59173][ML] Use isEmpty/nonEmpty instead of size comparisons in MLlib
### What changes were proposed in this pull request? This replaces size-vs-zero comparisons with the direct emptiness / non-emptiness predicates in the `mllib` module: - **Emptiness** (`x.size == 0` -> `x.isEmpty`): `TreeEnsembleModel.totalImportances` (an `OpenHashMap`, which `extends Iterable`) and two `Seq[ShuffleDependency]` checks in `ALSSuite`. - **Non-emptiness** (`x.size > 0` -> `x.nonEmpty`): `DecisionTreeMetadata.featureArity` and `Strategy.categoricalFeaturesInfo`, both Scala `Map[Int, Int]`. There are no Java-collection cases in this module. Deliberately left unchanged, because they are **not** collection-emptiness checks -- they are `mllib.linalg.Vector` dimensions or numeric sizes/thresholds, and `Vector` has no `isEmpty`/`nonEmpty`: - `ChiSqTest` (`expected` / `observed` are `Vector` dimensions, compared with `!= 0` and `== 0.0`); - `BinaryClassificationPMMLModelExport` and `GeneralizedLinearPMMLModelExport` (`model.weights`, a `Vector`); - `MultivariateOnlineSummarizer` (`instance`, a `Vector` -- the message reads "Vector should have dimension larger than zero"); - `BlockMatrix` (a matrix row `Vector`); - `ALS` (`RatingBlockBuilder.size`, a custom builder with no `nonEmpty`); - `SpearmanCorrelation` (`cachedUids.size >= 10000000`, a size threshold, not an emptiness check). ### Why are the changes needed? `isEmpty` / `nonEmpty` state the intent directly. Behavior is unchanged: every converted receiver (`OpenHashMap`, `Seq`, Scala `Map`) defines the predicate as exactly equivalent to the original comparison. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing tests. This is a behavior-preserving refactor; the `mllib` module compiles cleanly. Counterpart of SPARK-59149 (SQL) and SPARK-59172 (core). ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) Closes #58471 from uros-b/mllib-isempty. Authored-by: Uros <221401595+uros-b@users.noreply.github.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
1 parent db997b3 commit 0875765

4 files changed

Lines changed: 5 additions & 5 deletions

File tree

mllib/src/main/scala/org/apache/spark/ml/tree/impl/DecisionTreeMetadata.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ private[spark] class DecisionTreeMetadata(
6666

6767
def isMulticlass: Boolean = numClasses > 2
6868

69-
def isMulticlassWithCategoricalFeatures: Boolean = isMulticlass && (featureArity.size > 0)
69+
def isMulticlassWithCategoricalFeatures: Boolean = isMulticlass && (featureArity.nonEmpty)
7070

7171
def isCategorical(featureIndex: Int): Boolean = featureArity.contains(featureIndex)
7272

mllib/src/main/scala/org/apache/spark/ml/tree/treeModels.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ private[ml] object TreeEnsembleModel {
229229
maxFeatureIndex + 1
230230
}
231231
if (d == 0) {
232-
assert(totalImportances.size == 0, s"Unknown error in computing feature" +
232+
assert(totalImportances.isEmpty, s"Unknown error in computing feature" +
233233
s" importance: No splits found, but some non-zero importances.")
234234
}
235235
val (indices, values) = totalImportances.iterator.toSeq.sortBy(_._1).unzip

mllib/src/main/scala/org/apache/spark/mllib/tree/configuration/Strategy.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ class Strategy @Since("1.3.0") (
9898
*/
9999
@Since("1.2.0")
100100
def isMulticlassWithCategoricalFeatures: Boolean = {
101-
isMulticlassClassification && (categoricalFeaturesInfo.size > 0)
101+
isMulticlassClassification && (categoricalFeaturesInfo.nonEmpty)
102102
}
103103

104104
// scalastyle:off argcount

mllib/src/test/scala/org/apache/spark/ml/recommendation/ALSSuite.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -995,8 +995,8 @@ class ALSSuite extends MLTest with DefaultReadWriteTest with Logging {
995995
val shuffledItemFactors = getShuffledDependencies(itemFactors.rdd).filter { dep =>
996996
dep.rdd.name != null && dep.rdd.name.contains("itemFactors")
997997
}
998-
assert(shuffledUserFactors.size == 0)
999-
assert(shuffledItemFactors.size == 0)
998+
assert(shuffledUserFactors.isEmpty)
999+
assert(shuffledItemFactors.isEmpty)
10001000
}
10011001

10021002
private def checkRecommendations(

0 commit comments

Comments
 (0)