This is an automated email from the ASF dual-hosted git repository.

zhengruifeng 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 b142bab4e224 [SPARK-57910][ML][PYTHON] Make ALS reuse the shared 
HasIntermediateStorageLevel param
b142bab4e224 is described below

commit b142bab4e224cb1baeba57d957c72fc87076b2ae
Author: Mao Li <[email protected]>
AuthorDate: Tue Jul 7 08:42:16 2026 +0800

    [SPARK-57910][ML][PYTHON] Make ALS reuse the shared 
HasIntermediateStorageLevel param
    
    ### What changes were proposed in this pull request?
    
    This is a follow-up of SPARK-57860 
(https://github.com/apache/spark/pull/56949), as suggested by zhengruifeng 
during that review: now that the shared param trait 
`HasIntermediateStorageLevel` exists, make `ALS` reuse it instead of declaring 
its own duplicate `intermediateStorageLevel` param.
    
    - Scala: `ALSParams` now extends `HasIntermediateStorageLevel`; the 
locally-declared `intermediateStorageLevel` param, its getter, and its 
`setDefault` entry are removed (the shared trait provides all three with 
identical semantics: default `"MEMORY_AND_DISK"`, value cannot be `"NONE"`).
    - Python: `_ALSParams` now mixes in `HasIntermediateStorageLevel` from 
`pyspark.ml.param.shared`; the locally-declared `Param`, its getter, and its 
default are removed likewise.
    
    `ALS.setIntermediateStorageLevel` (Scala and Python) is untouched; 
`finalStorageLevel` stays local to ALS since it is ALS-specific.
    
    ### Why are the changes needed?
    
    Removes a duplicated param definition. The shared trait was deliberately 
modeled after ALS's existing param in SPARK-57860, so ALS itself should be its 
first reuse besides the new adopters. One definition to maintain, 
guaranteed-consistent validation and default.
    
    ### Does this PR introduce _any_ user-facing change?
    
    No behavior change. Param name, default (`"MEMORY_AND_DISK"`), validation 
(must be a valid `StorageLevel`, not `"NONE"`), getter, and setter are all 
unchanged.
    
    The only visible difference is the param's doc string, which now uses the 
shared trait's wording:
    - before: `StorageLevel for intermediate datasets. Cannot be 'NONE'.`
    - after: `StorageLevel for intermediate datasets. Pass in a string 
representation of `StorageLevel`. Cannot be 'NONE'.`
    
    ### How was this patch tested?
    
    Existing `ALSStorageSuite` already covers the param end-to-end (default 
level applied to intermediate RDDs, non-default level honored, `"NONE"`/invalid 
values rejected) and passes unchanged (all 4 `ml.recommendation` suites, 
38/38). `pyspark.ml.tests.test_param` (Scala/Python param parity via 
`test_java_params`) passes locally.
    
    MiMa reports two `FinalMethodProblem`s on `ALS.intermediateStorageLevel` / 
`ALS.getIntermediateStorageLevel`, because the members are declared `final` in 
the shared trait while ALS's local ones were not. Overriding a param `val` to 
change its semantics is not a supported usage, and the param's name, type, 
default and validation are unchanged, so this PR adds two narrowly-scoped 
`MimaExcludes` entries with an explaining comment.
    
    ### Was this patch authored or co-authored using generative AI tooling?
    
    Cooperate with: Claude Code (Fable 5)
    
    Closes #56979 from maoli67660/SPARK-57910.
    
    Authored-by: Mao Li <[email protected]>
    Signed-off-by: Ruifeng Zheng <[email protected]>
    (cherry picked from commit bf3bb385d7b952f4162452f5d677120111308fb4)
    Signed-off-by: Ruifeng Zheng <[email protected]>
---
 .../org/apache/spark/ml/recommendation/ALS.scala   | 17 +--------------
 project/MimaExcludes.scala                         |  9 +++++++-
 python/pyspark/ml/recommendation.py                | 24 ++++++++--------------
 3 files changed, 18 insertions(+), 32 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 1cee915046c0..43fa2c0ed3d2 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
@@ -139,7 +139,7 @@ private[recommendation] trait ALSModelParams extends Params 
with HasPredictionCo
  * Common params for ALS.
  */
 private[recommendation] trait ALSParams extends ALSModelParams with HasMaxIter 
with HasRegParam
-  with HasCheckpointInterval with HasSeed {
+  with HasCheckpointInterval with HasSeed with HasIntermediateStorageLevel {
 
   /**
    * Param for rank of the matrix factorization (positive).
@@ -215,20 +215,6 @@ private[recommendation] trait ALSParams extends 
ALSModelParams with HasMaxIter w
   /** @group getParam */
   def getNonnegative: Boolean = $(nonnegative)
 
-  /**
-   * Param for StorageLevel for intermediate datasets. Pass in a string 
representation of
-   * `StorageLevel`. Cannot be "NONE".
-   * Default: "MEMORY_AND_DISK".
-   *
-   * @group expertParam
-   */
-  val intermediateStorageLevel = new Param[String](this, 
"intermediateStorageLevel",
-    "StorageLevel for intermediate datasets. Cannot be 'NONE'.",
-    (s: String) => Try(StorageLevel.fromString(s)).isSuccess && s != "NONE")
-
-  /** @group expertGetParam */
-  def getIntermediateStorageLevel: String = $(intermediateStorageLevel)
-
   /**
    * Param for StorageLevel for ALS model factors. Pass in a string 
representation of
    * `StorageLevel`.
@@ -246,7 +232,6 @@ private[recommendation] trait ALSParams extends 
ALSModelParams with HasMaxIter w
   setDefault(rank -> 10, maxIter -> 10, regParam -> 0.1, numUserBlocks -> 10, 
numItemBlocks -> 10,
     implicitPrefs -> false, alpha -> 1.0, userCol -> "user", itemCol -> "item",
     ratingCol -> "rating", nonnegative -> false, checkpointInterval -> 10,
-    intermediateStorageLevel -> StorageLevelMapper.MEMORY_AND_DISK.name(),
     finalStorageLevel -> StorageLevelMapper.MEMORY_AND_DISK.name(), 
coldStartStrategy -> "nan")
 
   /**
diff --git a/project/MimaExcludes.scala b/project/MimaExcludes.scala
index 687311f1420f..43560c017014 100644
--- a/project/MimaExcludes.scala
+++ b/project/MimaExcludes.scala
@@ -34,7 +34,14 @@ import com.typesafe.tools.mima.core.*
 object MimaExcludes {
 
   // Exclude rules for 4.3.x from 4.2.0 (add 4.3-specific filters below as 
needed).
-  lazy val v43excludes: Seq[Problem => Boolean] = v42excludes
+  lazy val v43excludes: Seq[Problem => Boolean] = v42excludes ++ Seq(
+    // [SPARK-57910] ALS reuses the shared HasIntermediateStorageLevel param; 
the param and its
+    // getter are declared final in the shared trait (same name, type, default 
and validation).
+    ProblemFilters.exclude[FinalMethodProblem](
+      "org.apache.spark.ml.recommendation.ALS.intermediateStorageLevel"),
+    ProblemFilters.exclude[FinalMethodProblem](
+      "org.apache.spark.ml.recommendation.ALS.getIntermediateStorageLevel")
+  )
 
   // Exclude rules for 4.2.x from 4.1.0
   lazy val v42excludes = v41excludes ++ Seq(
diff --git a/python/pyspark/ml/recommendation.py 
b/python/pyspark/ml/recommendation.py
index 97a638ed6743..ee4a0c8d2e96 100644
--- a/python/pyspark/ml/recommendation.py
+++ b/python/pyspark/ml/recommendation.py
@@ -26,6 +26,7 @@ from pyspark.ml.param.shared import (
     HasRegParam,
     HasCheckpointInterval,
     HasSeed,
+    HasIntermediateStorageLevel,
 )
 from pyspark.ml.wrapper import JavaEstimator, JavaModel
 from pyspark.ml.common import inherit_doc
@@ -98,7 +99,14 @@ class _ALSModelParams(HasPredictionCol, HasBlockSize):
 
 
 @inherit_doc
-class _ALSParams(_ALSModelParams, HasMaxIter, HasRegParam, 
HasCheckpointInterval, HasSeed):
+class _ALSParams(
+    _ALSModelParams,
+    HasMaxIter,
+    HasRegParam,
+    HasCheckpointInterval,
+    HasSeed,
+    HasIntermediateStorageLevel,
+):
     """
     Params for :py:class:`ALS`.
 
@@ -145,12 +153,6 @@ class _ALSParams(_ALSModelParams, HasMaxIter, HasRegParam, 
HasCheckpointInterval
         "whether to use nonnegative constraint for least squares",
         typeConverter=TypeConverters.toBoolean,
     )
-    intermediateStorageLevel: Param[str] = Param(
-        Params._dummy(),
-        "intermediateStorageLevel",
-        "StorageLevel for intermediate datasets. Cannot be 'NONE'.",
-        typeConverter=TypeConverters.toString,
-    )
     finalStorageLevel: Param[str] = Param(
         Params._dummy(),
         "finalStorageLevel",
@@ -173,7 +175,6 @@ class _ALSParams(_ALSModelParams, HasMaxIter, HasRegParam, 
HasCheckpointInterval
             ratingCol="rating",
             nonnegative=False,
             checkpointInterval=10,
-            intermediateStorageLevel="MEMORY_AND_DISK",
             finalStorageLevel="MEMORY_AND_DISK",
             coldStartStrategy="nan",
         )
@@ -227,13 +228,6 @@ class _ALSParams(_ALSModelParams, HasMaxIter, HasRegParam, 
HasCheckpointInterval
         """
         return self.getOrDefault(self.nonnegative)
 
-    @since("2.0.0")
-    def getIntermediateStorageLevel(self) -> str:
-        """
-        Gets the value of intermediateStorageLevel or its default value.
-        """
-        return self.getOrDefault(self.intermediateStorageLevel)
-
     @since("2.0.0")
     def getFinalStorageLevel(self) -> str:
         """


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to