Raviston6296 commented on PR #60179:
URL: https://github.com/apache/spark/pull/60179#issuecomment-6689210734
Thanks, this looks good to me. I checked that the four classifiers have no
other hard-coded persist left: LR and LinearSVC only cache `blocks`,
FMClassifier only caches `data`, and MLP only caches in
`FeedForwardTrainer.train`, which is now wired up. The test also catches a
missed forward: without the `trainer.setIntermediateStorageLevel` line, MLP
would cache at `MEMORY_AND_DISK` and the `forall(_ == DISK_ONLY)` check would
fail. And because `ClassificationTestsMixin` is reused by
`ClassificationParityTests`, the new Python test also runs on Connect.
A few small points:
1. **Version tag:** master is 5.0.0-SNAPSHOT, but the new setters say
`@Since("4.4.0")` / `@since("4.4.0")`. That's right if this is going to
branch-4.x as well, like KMeans went to 4.3 (#59500 had to fix the tag on that
one afterwards). If it stays master-only, it should be 5.0.0.
2. **PySpark keyword arguments (optional):** the new param isn't in
`__init__`/`setParams`, so `LogisticRegression(maxBlockSizeInMB=1.0)` works but
`LogisticRegression(intermediateStorageLevel="DISK_ONLY")` raises `TypeError`.
That matches what KMeans does, so it's consistent, but ALS does accept it as a
keyword. It might be worth adding it for these four (and KMeans) at some point.
3. **Follow-up question:** `FMRegressor` has the same `handlePersistence` /
`persist(MEMORY_AND_DISK)` code as `FMClassifier`, and `LinearRegression` and
`AFTSurvivalRegression` cache their `InstanceBlock`s the same way LR and
LinearSVC do. Is a follow-up planned for the regressors?
--
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]
