mkincaid opened a new pull request, #56584:
URL: https://github.com/apache/spark/pull/56584

   ### What changes were proposed in this pull request?
   This patch unsets `parent` before calling the `SizeEstimator`.
   
   ### Why are the changes needed?
   Currently `SizeEstimator` includes the size of the `SparkSession` because it 
traverses the `parent` object which (in the case of many estimators that use 
DataFrame operations when fitting, like `StringIndexer`) eventually refers to 
the session. The session is there anyway and its size isn't attributable to 
fitting this specific model (and this results in double-counting when more 
models are fit), so it shouldn't be included in the size estimate.
   
   The impact of the bug is largest when the `SparkSession` is large. For 
example, in Databricks, my testing shows that a 300-800M SparkSession is 
typical. In some configurations, like Databricks serverless, the size limit for 
a single model object might be 256M, so this bug causes such models to fail to 
train regardless of the state of the cache otherwise.
   
   The Jira ticket includes a simple script that reproduces the condition 
locally, though the session is much smaller in that case (maybe 300k).
   
   ### Does this PR introduce _any_ user-facing change?
   Yes, a favorable one, in that the model cache would fill less quickly (and 
the reported sizes of cached models would be smaller, if they are among the 
affected models).
   
   ### How was this patch tested?
   A test is added: training a `StringIndexer` should estimate at no larger 
than 50k, in the trivial test case with 3 strings. This test fails before the 
patch and passes after it. Another similar test is provided for `MinMaxScaler`. 
A `ModelSuite` is added to hold these since the bug is at the `Model` level, 
not that of individual models (so the `StringIndexer` and `MinMaxScaler` suites 
aren't really the right place for these tests, although they are examples).
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Yes, the bug was discovered and initial patch/tests were created by pair 
programming with Claude. I wrote the bug/docs myself and validated the approach 
and final patch.
   
   Generated-by: Claude Opus 4.6


-- 
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]

Reply via email to