imatiach-msft commented on a change in pull request #21632:
[SPARK-19591][ML][MLlib] Add sample weights to decision trees
URL: https://github.com/apache/spark/pull/21632#discussion_r245549304
##########
File path: mllib/src/main/scala/org/apache/spark/ml/tree/impl/BaggedPoint.scala
##########
@@ -33,13 +33,20 @@ import org.apache.spark.util.random.XORShiftRandom
* this datum has 1 copy, 0 copies, and 4 copies in the 3 subsamples,
respectively.
*
* @param datum Data instance
- * @param subsampleWeights Weight of this instance in each subsampled dataset.
- *
- * TODO: This does not currently support (Double) weighted instances. Once
MLlib has weighted
- * dataset support, update. (We store subsampleWeights as Double for
this future extension.)
+ * @param subsampleCounts Number of samples of this instance in each
subsampled dataset.
+ * @param sampleWeight The weight of this instance.
*/
-private[spark] class BaggedPoint[Datum](val datum: Datum, val
subsampleWeights: Array[Double])
- extends Serializable
+private[spark] class BaggedPoint[Datum](
+ val datum: Datum,
+ val subsampleCounts: Array[Int],
+ val sampleWeight: Double) extends Serializable {
+
+ /**
+ * Subsample counts weighted by the sample weight.
+ */
+ def weightedCounts: Array[Double] = subsampleCounts.map(_ * sampleWeight)
Review comment:
I was going to say I'm not sure as well - as you said there is a small trade
off (also I would mention that for clarity in code I think it is a bit clearer
what the field is this way as opposed to doing the computation beforehand).
However, I noticed this field isn't even used anywhere, it's just there for
clarity, in here:
mllib/src/main/scala/org/apache/spark/ml/tree/impl/RandomForest.scala
we get a single subsample count and weight and they are passed separately to
another method orderedBinSeqOp or mixedBinSeqOp
I guess it's nice to have this method for clarity or debugging but if it's
not used maybe I should just get rid of it altogether? What are your thoughts
on this?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
