Github user jkbradley commented on a diff in the pull request:

    https://github.com/apache/spark/pull/2595#discussion_r18297130
  
    --- Diff: 
mllib/src/main/scala/org/apache/spark/mllib/tree/impl/DTStatsAggregator.scala 
---
    @@ -51,14 +71,19 @@ private[tree] abstract class DTStatsAggregator(
       def isUnordered(featureIndex: Int): Boolean = 
metadata.isUnordered(featureIndex)
     
       /**
    -   * Total number of elements stored in this aggregator.
    +   * Total number of elements stored in this aggregator
        */
    -  def allStatsSize: Int
    +  private val allStatsSize: Int = featureOffsets.last
     
       /**
    -   * Get flat array of elements stored in this aggregator.
    +   * Flat array of elements.
    +   * Index for start of stats for a (feature, bin) is:
    +   *   index = featureOffsets(featureIndex) + binIndex * statsSize
    +   * Note: For unordered features, the left child stats have binIndex in 
[0, numBins(featureIndex))
    --- End diff --
    
    Error in doc (not code):
    The left & right child stats are actually in:
    [0, numBins(featureIndex) / 2) and
    [numBins(featureIndex) / 2, numBins(featureIndex))
    The semantics of "numBins" was updated in a previous PR so that 1 "bin" 
holds the same number of values for any type of feature.  (You can see the 
update in DecisionTreeMetadata.)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to