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

    https://github.com/apache/spark/pull/2125#discussion_r16865586
  
    --- Diff: 
mllib/src/main/scala/org/apache/spark/mllib/tree/impl/DecisionTreeMetadata.scala
 ---
    @@ -70,32 +83,48 @@ private[tree] object DecisionTreeMetadata {
           case Regression => 0
         }
     
    -    val maxBins = math.min(strategy.maxBins, numExamples).toInt
    -    val log2MaxBinsp1 = math.log(maxBins + 1) / math.log(2.0)
    +    val maxPossibleBins = math.min(strategy.maxBins, numExamples).toInt
    +    val log2MaxPossibleBinsp1 = math.log(maxPossibleBins + 1) / 
math.log(2.0)
     
    +    // We check the number of bins here against maxPossibleBins.
    +    // This needs to be checked here instead of in Strategy since 
maxPossibleBins can be modified
    +    // based on the number of training examples.
         val unorderedFeatures = new mutable.HashSet[Int]()
    +    val numBins = Array.fill[Int](numFeatures)(maxPossibleBins)
         if (numClasses > 2) {
           strategy.categoricalFeaturesInfo.foreach { case (f, k) =>
    -        if (k - 1 < log2MaxBinsp1) {
    +        if (k - 1 < log2MaxPossibleBinsp1) {
               // Note: The above check is equivalent to checking:
               //       numUnorderedBins = (1 << k - 1) - 1 < maxBins
               unorderedFeatures.add(f)
    +          numBins(f) = numUnorderedBins(k)
             } else {
    -          // TODO: Allow this case, where we simply will know nothing 
about some categories?
    -          require(k < maxBins, s"maxBins (= $maxBins) should be greater 
than max categories " +
    +          require(k <= maxPossibleBins,
    +            s"maxBins (= $maxPossibleBins) should be greater than max 
categories " +
                 s"in categorical features (>= $k)")
    +          numBins(f) = k
             }
           }
         } else {
           strategy.categoricalFeaturesInfo.foreach { case (f, k) =>
    -        require(k < maxBins, s"maxBins (= $maxBins) should be greater than 
max categories " +
    -          s"in categorical features (>= $k)")
    +        require(k <= maxPossibleBins,
    +          s"DecisionTree requires maxBins (= $maxPossibleBins) >= max 
categories " +
    +          s"in categorical features (= 
${strategy.categoricalFeaturesInfo.values.max})")
    --- End diff --
    
    could extract strategy.categoricalFeaturesInfo into a val for reuse.


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