Re: [PR] [SPARK-58279][ML][CONNECT] Estimate size of TargetEncoder, VectorIndexer, CountVectorizer, and MinHashLSH models [spark]

["uros-b (via GitHub)" <gi...@apache.org>]

uros-b commented on code in PR #57448:
URL: https://github.com/apache/spark/pull/57448#discussion_r3637517012


##########
mllib/src/main/scala/org/apache/spark/ml/feature/VectorIndexer.scala:
##########
@@ -302,6 +303,13 @@ class VectorIndexerModel private[ml] (
   // For ml connect only
   private[ml] def this() = this("", -1, Map.empty)
 
+  private[spark] override def estimatedSize: Long = {

Review Comment:
   Note: VectorIndexerModel.estimatedSize counts only categoryMaps and omits partialFeatureAttributes, an eager private val (not lazy) built in the primary constructor as an Array[Attribute] of numFeatures allocated attribute objects. Because the old default estimated a parentless copy (new VectorIndexerModel(uid, numFeatures, categoryMaps), which re-materializes partialFeatureAttributes), that array was previously counted and is now dropped from the estimate. numFeatures is not bounded by categoryMaps.size, so a high-dimensional model (e.g. numFeatures=1000, few categorical) under-counts by ~1000 attribute objects. The merged sibling StringIndexerModel (SPARK-58251) sets the precedent by explicitly counting its derived eager field: SizeEstimator.estimate((labelsArray, labelsToIndexArray)). Fix is one line: estimateMatadataSize + SizeEstimator.estimate((categoryMaps, partialFeatureAttributes)).



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org