chinaoel opened a new pull request, #59353: URL: https://github.com/apache/spark/pull/59353
### What changes were proposed in this pull request? Fix `CrossValidatorModelReader.load` in `python/pyspark/ml/tuning.py` so that each fold gets its own list of sub-models: ```python # before subModels = [[None] * len(estimatorParamMaps)] * numFolds # after subModels = [[None] * len(estimatorParamMaps) for _ in range(numFolds)] ``` `[x] * n` repeats a reference to the same inner list, so every fold shared one list and each fold's assignment overwrote the previous one. `CrossValidator._fit`, `CrossValidatorModel.copy`, `CrossValidatorModel._from_java` and the Scala reader (`Array.fill(numFolds)(...)`) already build an independent list per fold. The existing test in `test_tuning.py` compared `model.subModels[i][j].uid` with itself, so it always passed. After loading it only checked list lengths. This PR replaces that check with a comparison of each loaded sub-model's coefficients and intercept against the original. Comparing uids would not catch this bug either, because all sub-models share the estimator's uid. ### Why are the changes needed? After `CrossValidatorModel.load`, every entry of `subModels` returns the last fold's models. The files on disk are correct; only the PySpark reader is affected. Saving a loaded model again writes the last fold's models into every fold directory, so the original sub-models are lost. `bestModel` and `avgMetrics` are not affected. Reproduction (`numFolds=3`, 2 param maps, `collectSubModels=True`), first coefficient of each sub-model: | | fold0 | fold1 | fold2 | |---|---|---|---| | after `fit` | [0.0705, 0.0439] | [0.0067, 0.0047] | [-0.0155, -0.0108] | | on disk after `save` | [0.0705, 0.0439] | [0.0067, 0.0047] | [-0.0155, -0.0108] | | after `CrossValidatorModel.load` | [-0.0155, -0.0108] | [-0.0155, -0.0108] | [-0.0155, -0.0108] | `loaded.subModels[0] is loaded.subModels[1]` returns `True`. The bug was introduced by SPARK-33520. ### Does this PR introduce _any_ user-facing change? Yes, as a bug fix. `CrossValidatorModel.load` now returns the correct sub-models for each fold. Previously every fold returned the last fold's sub-models. ### How was this patch tested? Updated `TuningTestsMixin.test_cross_validator` to check that the loaded sub-models match the original ones. The test fails without the fix (`AssertionError: 0.6466271649250525 != 0.10536051565782635` on the intercept) and passes with it. The mixin is also used by the Spark Connect parity test. ``` python/run-tests --testnames 'pyspark.ml.tests.test_tuning,pyspark.ml.tests.tuning.test_tuning,pyspark.ml.tests.tuning.test_cv_io_basic,pyspark.ml.tests.tuning.test_cv_io_nested,pyspark.ml.tests.tuning.test_cv_io_pipeline,pyspark.ml.tests.tuning.test_tvs_io_basic,pyspark.ml.tests.connect.test_parity_tuning,pyspark.ml.tests.connect.test_legacy_mode_tuning' ``` ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5.5) 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
