[SPARK-58253][ML][CONNECT] Add size estimates for RobustScalerModel, MinMaxScalerModel, MaxAbsScalerModel, and StandardScalerModel#57424
Conversation
zhengruifeng
changed the title
|
Thank you @zhengruifeng and @HyukjinKwon! |
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
AFAIK, the PR description seems outdated. The parent-traversal issue described here was already fixed generically by SPARK-57521, which made the default Model.estimatedSize estimate a parentless copy of the model.
Fitted models retain their parent estimator. Estimating the whole reachable object graph can therefore count shared SparkSession or SparkContext state instead of only the model footprint.
So the actual contribution of this PR is rather: (a) a cheaper and more precise estimation that avoids the reflection-based SizeEstimator.estimate graph walk, and (b) consistency with the other models that already provide explicit estimatedSize implementations (e.g., FMClassificationModel, LinearSVCModel). Could you update the PR description accordingly, @zhengruifeng ?
|
@dongjoon-hyun Thank you for the clarification. I updated the PR description to note that SPARK-57521 already handles parent traversal, and that this PR provides direct, more precise size accounting for these models. |
…MinMaxScalerModel, MaxAbsScalerModel, and StandardScalerModel ### What changes were proposed in this pull request? Add explicit `estimatedSize` implementations for RobustScalerModel, MinMaxScalerModel, MaxAbsScalerModel, and StandardScalerModel. Each directly accounts for model metadata and its learned vectors via `getSizeInBytes`, rather than using a reflection-based object-graph walk. ### Why are the changes needed? SPARK-57521 already makes the default `Model.estimatedSize` estimate a parentless copy, avoiding parent-estimator and SparkSession traversal. These explicit implementations make size accounting cheaper and more precise for scalar models, and align them with models that already provide specialized estimates. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? - Added fitted-model size bounds to the corresponding feature algorithm suites. - `JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64 build/sbt 'mllib/Test/compile'`\n- Targeted suites were not run.\n\n### Was this patch authored or co-authored using generative AI tooling?\n\nGenerated-by: OpenAI Codex (GPT-5) Closes #57424 from zhengruifeng/scalar-model-size-estimates-master-dev3. Authored-by: Ruifeng Zheng <ruifengz@apache.org> Signed-off-by: Ruifeng Zheng <ruifengz@apache.org> (cherry picked from commit 6daabb8) Signed-off-by: Ruifeng Zheng <ruifengz@apache.org>
|
Thank you, @zhengruifeng . |
4 participants
What changes were proposed in this pull request?
Add explicit
estimatedSizeimplementations for RobustScalerModel, MinMaxScalerModel, MaxAbsScalerModel, and StandardScalerModel. Each directly accounts for model metadata and its learned vectors viagetSizeInBytes, rather than using a reflection-based object-graph walk.Why are the changes needed?
SPARK-57521 already makes the default
Model.estimatedSizeestimate a parentless copy, avoiding parent-estimator and SparkSession traversal. These explicit implementations make size accounting cheaper and more precise for scalar models, and align them with models that already provide specialized estimates.Does this PR introduce any user-facing change?
No.
How was this patch tested?
JAVA_HOME=/usr/lib/jvm/java-17-openjdk-amd64 build/sbt 'mllib/Test/compile'\n- Targeted suites were not run.\n\n### Was this patch authored or co-authored using generative AI tooling?\n\nGenerated-by: OpenAI Codex (GPT-5)