[mllib] DecisionTree: treeAggregate + Python example bug fix #2015

jkbradley · 2014-08-18T15:12:57Z

Small DecisionTree updates:

Changed main DecisionTree aggregate to treeAggregate.
Fixed bug in python example decision_tree_runner.py with missing argument (since categoricalFeaturesInfo is no longer an optional argument for trainClassifier).
Fixed same bug in python doc tests, and added tree.py to doc tests.

Conflicts: mllib/src/main/scala/org/apache/spark/mllib/tree/DecisionTree.scala

* Added TreePoint representation to avoid calling findBin multiple times. * (not working yet, but debugging)

Optimization: Added TreePoint representation so we only call findBin once for each example, feature. Also, calculateGainsForAllNodeSplits now only searches over actual splits, not empty/unused ones. BUG FIX: isSampleValid * isSampleValid used to treat unordered categorical features incorrectly: It treated the bins as if indexed by featured values, rather than by subsets of values/categories. * exhibited for unordered features (multi-class classification with categorical features of low arity) * Fix: Index bins correctly for unordered categorical features. Also: some commented-out debugging println calls in DecisionTree, to be removed later

DecisionTree.scala * Eliminated filters, replaced by building tree on the fly and filtering top-down. ** Aggregation over examples now skips examples which do not reach the current level. * Only calculate unorderedFeatures once (in findSplitsBins) Node: Renamed predictIfLeaf to predict Bin, Split: Updated doc

* Simplification: Updated calculateGainForSplit to take aggregates for a single (feature, split) pair. * Internal doc: findAggForOrderedFeatureClassification

…emoved debugging println calls from DecisionTree. Made TreePoint extend Serialiable

* Updated doc * Made some methods private Changed timer to report time in seconds.

Conflicts: mllib/src/main/scala/org/apache/spark/mllib/tree/DecisionTree.scala mllib/src/test/scala/org/apache/spark/mllib/tree/DecisionTreeSuite.scala

…ng println calls in DecisionTree.scala.

…disk, not just memory. Details: DecisionTree * Changed: .cache() -> .persist(StorageLevel.MEMORY_AND_DISK) ** This gave major performance improvements on small tests. E.g., 500K examples, 500 features, depth 5, on MacBook, took 292 sec with cache() and 112 when using disk as well. * Change for to while loops * Small cleanups TimeTracker * Removed useless timing in DecisionTree TreePoint * Renamed features to binnedFeatures

Conflicts: mllib/src/main/scala/org/apache/spark/mllib/tree/DecisionTree.scala mllib/src/test/scala/org/apache/spark/mllib/tree/DecisionTreeSuite.scala Merge is OK except one DT Suite test to fix.

…te treePointToNodeIndex with groupShift.

Main change: Now using << instead of math.pow.

…ment. Changed main DecisionTree aggregate to treeAggregate.

…FeaturesInfo argument.

jkbradley · 2014-08-18T16:01:36Z

Jenkins, test this please.

SparkQA · 2014-08-18T16:05:18Z

QA tests have started for PR 2015 at commit 8e4665d.

This patch merges cleanly.

mengxr · 2014-08-18T16:54:44Z

examples/src/main/python/mllib/decision_tree_runner.py

= -> = (spaces around =)

SparkQA · 2014-08-18T16:55:58Z

QA tests have finished for PR 2015 at commit 8e4665d.

This patch fails unit tests.
This patch merges cleanly.
This patch adds no public classes.

mengxr · 2014-08-18T16:57:53Z

It seems that we put an extra empty line at the end of TreeModel.toString().

jkbradley · 2014-08-18T18:56:44Z

@mengxr Uploaded fix. I kept the newline at the end of the toString() method since it makes sense to ensure a newline there, and removed one newline in the doctest.

SparkQA · 2014-08-18T19:00:27Z

QA tests have started for PR 2015 at commit b5114fa.

This patch merges cleanly.

SparkQA · 2014-08-18T19:53:48Z

QA tests have finished for PR 2015 at commit b5114fa.

This patch passes unit tests.
This patch merges cleanly.
This patch adds no public classes.

mengxr · 2014-08-18T21:40:47Z

LGTM. Merged into master and branch-1.1. Thanks!

Small DecisionTree updates: * Changed main DecisionTree aggregate to treeAggregate. * Fixed bug in python example decision_tree_runner.py with missing argument (since categoricalFeaturesInfo is no longer an optional argument for trainClassifier). * Fixed same bug in python doc tests, and added tree.py to doc tests. CC: mengxr Author: Joseph K. Bradley <joseph.kurata.bradley@gmail.com> Closes #2015 from jkbradley/dt-opt2 and squashes the following commits: b5114fa [Joseph K. Bradley] Fixed python tree.py doc test (extra newline) 8e4665d [Joseph K. Bradley] Added tree.py to python doc tests. Fixed bug from missing categoricalFeaturesInfo argument. b7b2922 [Joseph K. Bradley] Fixed bug in python example decision_tree_runner.py with missing argument. Changed main DecisionTree aggregate to treeAggregate. 85bbc1f [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-opt2 66d076f [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-opt2 a0ed0da [Joseph K. Bradley] Renamed DTMetadata to DecisionTreeMetadata. Small doc updates. 3726d20 [Joseph K. Bradley] Small code improvements based on code review. ac0b9f8 [Joseph K. Bradley] Small updates based on code review. Main change: Now using << instead of math.pow. db0d773 [Joseph K. Bradley] scala style fix 6a38f48 [Joseph K. Bradley] Added DTMetadata class for cleaner code 931a3a7 [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-opt2 797f68a [Joseph K. Bradley] Fixed DecisionTreeSuite bug for training second level. Needed to update treePointToNodeIndex with groupShift. f40381c [Joseph K. Bradley] Merge branch 'dt-opt1' into dt-opt2 5f2dec2 [Joseph K. Bradley] Fixed scalastyle issue in TreePoint 6b5651e [Joseph K. Bradley] Updates based on code review. 1 major change: persisting to memory + disk, not just memory. 2d2aaaf [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-opt1 26d10dd [Joseph K. Bradley] Removed tree/model/Filter.scala since no longer used. Removed debugging println calls in DecisionTree.scala. 356daba [Joseph K. Bradley] Merge branch 'dt-opt1' into dt-opt2 430d782 [Joseph K. Bradley] Added more debug info on binning error. Added some docs. d036089 [Joseph K. Bradley] Print timing info to logDebug. e66f1b1 [Joseph K. Bradley] TreePoint * Updated doc * Made some methods private 8464a6e [Joseph K. Bradley] Moved TimeTracker to tree/impl/ in its own file, and cleaned it up. Removed debugging println calls from DecisionTree. Made TreePoint extend Serialiable a87e08f [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-opt1 c1565a5 [Joseph K. Bradley] Small DecisionTree updates: * Simplification: Updated calculateGainForSplit to take aggregates for a single (feature, split) pair. * Internal doc: findAggForOrderedFeatureClassification b914f3b [Joseph K. Bradley] DecisionTree optimization: eliminated filters + small changes b2ed1f3 [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-opt 0f676e2 [Joseph K. Bradley] Optimizations + Bug fix for DecisionTree 3211f02 [Joseph K. Bradley] Optimizing DecisionTree * Added TreePoint representation to avoid calling findBin multiple times. * (not working yet, but debugging) f61e9d2 [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-timing bcf874a [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-timing 511ec85 [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-timing a95bc22 [Joseph K. Bradley] timing for DecisionTree internals (cherry picked from commit 115eeb3) Signed-off-by: Xiangrui Meng <meng@databricks.com>

Small DecisionTree updates: * Changed main DecisionTree aggregate to treeAggregate. * Fixed bug in python example decision_tree_runner.py with missing argument (since categoricalFeaturesInfo is no longer an optional argument for trainClassifier). * Fixed same bug in python doc tests, and added tree.py to doc tests. CC: mengxr Author: Joseph K. Bradley <joseph.kurata.bradley@gmail.com> Closes apache#2015 from jkbradley/dt-opt2 and squashes the following commits: b5114fa [Joseph K. Bradley] Fixed python tree.py doc test (extra newline) 8e4665d [Joseph K. Bradley] Added tree.py to python doc tests. Fixed bug from missing categoricalFeaturesInfo argument. b7b2922 [Joseph K. Bradley] Fixed bug in python example decision_tree_runner.py with missing argument. Changed main DecisionTree aggregate to treeAggregate. 85bbc1f [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-opt2 66d076f [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-opt2 a0ed0da [Joseph K. Bradley] Renamed DTMetadata to DecisionTreeMetadata. Small doc updates. 3726d20 [Joseph K. Bradley] Small code improvements based on code review. ac0b9f8 [Joseph K. Bradley] Small updates based on code review. Main change: Now using << instead of math.pow. db0d773 [Joseph K. Bradley] scala style fix 6a38f48 [Joseph K. Bradley] Added DTMetadata class for cleaner code 931a3a7 [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-opt2 797f68a [Joseph K. Bradley] Fixed DecisionTreeSuite bug for training second level. Needed to update treePointToNodeIndex with groupShift. f40381c [Joseph K. Bradley] Merge branch 'dt-opt1' into dt-opt2 5f2dec2 [Joseph K. Bradley] Fixed scalastyle issue in TreePoint 6b5651e [Joseph K. Bradley] Updates based on code review. 1 major change: persisting to memory + disk, not just memory. 2d2aaaf [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-opt1 26d10dd [Joseph K. Bradley] Removed tree/model/Filter.scala since no longer used. Removed debugging println calls in DecisionTree.scala. 356daba [Joseph K. Bradley] Merge branch 'dt-opt1' into dt-opt2 430d782 [Joseph K. Bradley] Added more debug info on binning error. Added some docs. d036089 [Joseph K. Bradley] Print timing info to logDebug. e66f1b1 [Joseph K. Bradley] TreePoint * Updated doc * Made some methods private 8464a6e [Joseph K. Bradley] Moved TimeTracker to tree/impl/ in its own file, and cleaned it up. Removed debugging println calls from DecisionTree. Made TreePoint extend Serialiable a87e08f [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-opt1 c1565a5 [Joseph K. Bradley] Small DecisionTree updates: * Simplification: Updated calculateGainForSplit to take aggregates for a single (feature, split) pair. * Internal doc: findAggForOrderedFeatureClassification b914f3b [Joseph K. Bradley] DecisionTree optimization: eliminated filters + small changes b2ed1f3 [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-opt 0f676e2 [Joseph K. Bradley] Optimizations + Bug fix for DecisionTree 3211f02 [Joseph K. Bradley] Optimizing DecisionTree * Added TreePoint representation to avoid calling findBin multiple times. * (not working yet, but debugging) f61e9d2 [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-timing bcf874a [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-timing 511ec85 [Joseph K. Bradley] Merge remote-tracking branch 'upstream/master' into dt-timing a95bc22 [Joseph K. Bradley] timing for DecisionTree internals

jkbradley added 30 commits August 5, 2014 11:17

timing for DecisionTree internals

a95bc22

Merge remote-tracking branch 'upstream/master' into dt-timing

511ec85

Merge remote-tracking branch 'upstream/master' into dt-timing

bcf874a

Conflicts: mllib/src/main/scala/org/apache/spark/mllib/tree/DecisionTree.scala

Merge remote-tracking branch 'upstream/master' into dt-timing

f61e9d2

Optimizing DecisionTree

3211f02

* Added TreePoint representation to avoid calling findBin multiple times. * (not working yet, but debugging)

Merge remote-tracking branch 'upstream/master' into dt-opt

b2ed1f3

Small DecisionTree updates:

c1565a5

* Simplification: Updated calculateGainForSplit to take aggregates for a single (feature, split) pair. * Internal doc: findAggForOrderedFeatureClassification

Merge remote-tracking branch 'upstream/master' into dt-opt1

a87e08f

Moved TimeTracker to tree/impl/ in its own file, and cleaned it up. R…

8464a6e

…emoved debugging println calls from DecisionTree. Made TreePoint extend Serialiable

TreePoint

e66f1b1

* Updated doc * Made some methods private Changed timer to report time in seconds.

Print timing info to logDebug.

d036089

Added more debug info on binning error. Added some docs.

430d782

Merge branch 'dt-opt1' into dt-opt2

356daba

Conflicts: mllib/src/main/scala/org/apache/spark/mllib/tree/DecisionTree.scala mllib/src/test/scala/org/apache/spark/mllib/tree/DecisionTreeSuite.scala

Removed tree/model/Filter.scala since no longer used. Removed debuggi…

26d10dd

…ng println calls in DecisionTree.scala.

Merge remote-tracking branch 'upstream/master' into dt-opt1

2d2aaaf

Fixed scalastyle issue in TreePoint

5f2dec2

Merge branch 'dt-opt1' into dt-opt2

f40381c

Conflicts: mllib/src/main/scala/org/apache/spark/mllib/tree/DecisionTree.scala mllib/src/test/scala/org/apache/spark/mllib/tree/DecisionTreeSuite.scala Merge is OK except one DT Suite test to fix.

Fixed DecisionTreeSuite bug for training second level. Needed to upda…

797f68a

…te treePointToNodeIndex with groupShift.

Merge remote-tracking branch 'upstream/master' into dt-opt2

931a3a7

Added DTMetadata class for cleaner code

6a38f48

scala style fix

db0d773

Small updates based on code review.

ac0b9f8

Main change: Now using << instead of math.pow.

Small code improvements based on code review.

3726d20

Renamed DTMetadata to DecisionTreeMetadata. Small doc updates.

a0ed0da

Merge remote-tracking branch 'upstream/master' into dt-opt2

66d076f

Merge remote-tracking branch 'upstream/master' into dt-opt2

85bbc1f

Fixed bug in python example decision_tree_runner.py with missing argu…

b7b2922

…ment. Changed main DecisionTree aggregate to treeAggregate.

Added tree.py to python doc tests. Fixed bug from missing categorical…

8e4665d

…FeaturesInfo argument.

mengxr reviewed Aug 18, 2014
View reviewed changes

examples/src/main/python/mllib/decision_tree_runner.py

Copy link

Contributor

mengxr Aug 18, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

= -> = (spaces around =)

Fixed python tree.py doc test (extra newline)

b5114fa

asfgit closed this in 115eeb3 Aug 18, 2014

jkbradley deleted the dt-opt2 branch August 26, 2014 17:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[mllib] DecisionTree: treeAggregate + Python example bug fix #2015

[mllib] DecisionTree: treeAggregate + Python example bug fix #2015

Uh oh!

jkbradley commented Aug 18, 2014

Uh oh!

jkbradley commented Aug 18, 2014

Uh oh!

SparkQA commented Aug 18, 2014

Uh oh!

mengxr Aug 18, 2014

Uh oh!

SparkQA commented Aug 18, 2014

Uh oh!

mengxr commented Aug 18, 2014

Uh oh!

jkbradley commented Aug 18, 2014

Uh oh!

SparkQA commented Aug 18, 2014

Uh oh!

SparkQA commented Aug 18, 2014

Uh oh!

mengxr commented Aug 18, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[mllib] DecisionTree: treeAggregate + Python example bug fix #2015

[mllib] DecisionTree: treeAggregate + Python example bug fix #2015

Uh oh!

Conversation

jkbradley commented Aug 18, 2014

Uh oh!

jkbradley commented Aug 18, 2014

Uh oh!

SparkQA commented Aug 18, 2014

Uh oh!

mengxr Aug 18, 2014

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 18, 2014

Uh oh!

mengxr commented Aug 18, 2014

Uh oh!

jkbradley commented Aug 18, 2014

Uh oh!

SparkQA commented Aug 18, 2014

Uh oh!

SparkQA commented Aug 18, 2014

Uh oh!

mengxr commented Aug 18, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants