{"action":"created","comment":{"url":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/comments/39718142","id":39718142,"diff_hunk":"@@ -1096,45 +949,41 @@ cdef class FriedmanMSE(MSE):\n         The absolute impurity improvement is only computed by the\n         impurity_improvement method once the best split has been found.\n         \"\"\"\n-        cdef SIZE_t n_outputs = self.n_outputs\n-        cdef double* sum_left = self.sum_left\n-        cdef double* sum_right = self.sum_right\n-        cdef double weighted_n_left = self.weighted_n_left\n-        cdef double weighted_n_right = self.weighted_n_right\n+\n+        cdef double* node_sum_left = self.node_sum_left\n+        cdef double* node_sum_right = self.node_sum_right\n+\n         cdef double total_sum_left = 0.0\n         cdef double total_sum_right = 0.0\n \n-        cdef SIZE_t k = 0\n+        cdef SIZE_t k\n         cdef double diff = 0.0\n \n-        for k in range(n_outputs):\n-            total_sum_left += sum_left[k]\n-            total_sum_right += sum_right[k]\n+        for k in range(self.n_outputs):\n+            total_sum_left += node_sum_left[k]\n+            total_sum_right += node_sum_right[k]\n \n-        diff = (weighted_n_right * total_sum_left -\n-                weighted_n_left * total_sum_right)\n+        diff = (self.weighted_n_right * total_sum_left -\n+                self.weighted_n_left * total_sum_right)\n \n-        return diff * diff / (weighted_n_left * weighted_n_right)\n+        return diff * diff / (self.weighted_n_left * self.weighted_n_right)\n \n     cdef double impurity_improvement(self, double impurity) nogil:\n-        cdef SIZE_t n_outputs = self.n_outputs\n-        cdef double* sum_left = self.sum_left\n-        cdef double* sum_right = self.sum_right\n-        cdef double weighted_n_left = self.weighted_n_left\n-        cdef double weighted_n_right = self.weighted_n_right\n-        cdef double weighted_n_node_samples = self.weighted_n_node_samples\n+        cdef double* node_sum_left = self.node_sum_left\n+        cdef double* node_sum_right = self.node_sum_right\n+\n         cdef double total_sum_left = 0.0\n         cdef double total_sum_right = 0.0\n \n-        cdef SIZE_t k = 0\n+        cdef SIZE_t k\n         cdef double diff = 0.0\n \n-        for k in range(n_outputs):\n-            total_sum_left += sum_left[k]\n-            total_sum_right += sum_right[k]\n+        for k in range(self.n_outputs):\n+            total_sum_left += node_sum_left[k]\n+            total_sum_right += node_sum_right[k]\n \n-        diff = (weighted_n_right * total_sum_left -\n-                weighted_n_left * total_sum_right) / n_outputs\n+        diff = (self.weighted_n_right * total_sum_left -\n+                self.weighted_n_left * total_sum_right) / self.n_outputs\n \n-        return (diff * diff /\n-                (weighted_n_left * weighted_n_right * weighted_n_node_samples))\n\\ No newline at end of file\n+        return (diff * diff / (self.weighted_n_left * self.weighted_n_right * \n+                self.weighted_n_node_samples))","path":"sklearn/tree/_criterion.pyx","position":1013,"original_position":1013,"commit_id":"201eb23b119ad0a7e687a3f7d3204be09af8d1f6","original_commit_id":"201eb23b119ad0a7e687a3f7d3204be09af8d1f6","user":{"login":"glouppe","id":477771,"avatar_url":"https://avatars.githubusercontent.com/u/477771?v=3","gravatar_id":"","url":"https://api.github.com/users/glouppe","html_url":"https://github.com/glouppe","followers_url":"https://api.github.com/users/glouppe/followers","following_url":"https://api.github.com/users/glouppe/following{/other_user}","gists_url":"https://api.github.com/users/glouppe/gists{/gist_id}","starred_url":"https://api.github.com/users/glouppe/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/glouppe/subscriptions","organizations_url":"https://api.github.com/users/glouppe/orgs","repos_url":"https://api.github.com/users/glouppe/repos","events_url":"https://api.github.com/users/glouppe/events{/privacy}","received_events_url":"https://api.github.com/users/glouppe/received_events","type":"User","site_admin":false},"body":"Many older tools may misbehave if the last line of a text file is not terminated by a line return. (Because otherwise the last line of context contains EOF and may be ignored)","created_at":"2015-09-17T07:42:42Z","updated_at":"2015-09-17T07:42:42Z","html_url":"https://github.com/scikit-learn/scikit-learn/pull/5278#discussion_r39718142","pull_request_url":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/5278","_links":{"self":{"href":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/comments/39718142"},"html":{"href":"https://github.com/scikit-learn/scikit-learn/pull/5278#discussion_r39718142"},"pull_request":{"href":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/5278"}}},"pull_request":{"url":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/5278","id":45155173,"html_url":"https://github.com/scikit-learn/scikit-learn/pull/5278","diff_url":"https://github.com/scikit-learn/scikit-learn/pull/5278.diff","patch_url":"https://github.com/scikit-learn/scikit-learn/pull/5278.patch","issue_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/5278","number":5278,"state":"open","locked":false,"title":"[MRG] _criterion.pyx cleanup","user":{"login":"jmschrei","id":3916816,"avatar_url":"https://avatars.githubusercontent.com/u/3916816?v=3","gravatar_id":"","url":"https://api.github.com/users/jmschrei","html_url":"https://github.com/jmschrei","followers_url":"https://api.github.com/users/jmschrei/followers","following_url":"https://api.github.com/users/jmschrei/following{/other_user}","gists_url":"https://api.github.com/users/jmschrei/gists{/gist_id}","starred_url":"https://api.github.com/users/jmschrei/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jmschrei/subscriptions","organizations_url":"https://api.github.com/users/jmschrei/orgs","repos_url":"https://api.github.com/users/jmschrei/repos","events_url":"https://api.github.com/users/jmschrei/events{/privacy}","received_events_url":"https://api.github.com/users/jmschrei/received_events","type":"User","site_admin":false},"body":"This pull request focuses on cleaning up criterion code, without adding any new functionality. This is the first step to adding caching between splits. That branch had too many changes, so I broke it up to be easier to review. This PR addresses the following:\r\n\r\n(1) Eliminates nearly 200 lines of unneeded code (mostly by calling class variables `self.n_outputs` directly instead of unpacking them)\r\n(2) Standardizes `label_count` for classification criteria and `sum_total` for regression into the `node_sum` (repeat for left and right versions)\r\n(3) Adds `yw_sq_sum` as the last sufficient statistic needed for regression criteria between splits\r\n(4) Previously, regression `child_impurity` recalculated sufficient statistics by rescanning the sequence, despite them already being present. I fixed this. However, the speed increase is minimal given how rarely this function is called.\r\n\r\nAll unit tests pass, and speed is basically the same as before.  \r\n\r\nping @arjoly @glouppe @ogrisel ","created_at":"2015-09-16T10:33:39Z","updated_at":"2015-09-17T07:42:42Z","closed_at":null,"merged_at":null,"merge_commit_sha":"f4d1ed70aa12453132518ca42cc657a917bea61a","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/5278/commits","review_comments_url":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/5278/comments","review_comment_url":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/comments{/number}","comments_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/5278/comments","statuses_url":"https://api.github.com/repos/scikit-learn/scikit-learn/statuses/201eb23b119ad0a7e687a3f7d3204be09af8d1f6","head":{"label":"jmschrei:_criterion_cleanup","ref":"_criterion_cleanup","sha":"201eb23b119ad0a7e687a3f7d3204be09af8d1f6","user":{"login":"jmschrei","id":3916816,"avatar_url":"https://avatars.githubusercontent.com/u/3916816?v=3","gravatar_id":"","url":"https://api.github.com/users/jmschrei","html_url":"https://github.com/jmschrei","followers_url":"https://api.github.com/users/jmschrei/followers","following_url":"https://api.github.com/users/jmschrei/following{/other_user}","gists_url":"https://api.github.com/users/jmschrei/gists{/gist_id}","starred_url":"https://api.github.com/users/jmschrei/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jmschrei/subscriptions","organizations_url":"https://api.github.com/users/jmschrei/orgs","repos_url":"https://api.github.com/users/jmschrei/repos","events_url":"https://api.github.com/users/jmschrei/events{/privacy}","received_events_url":"https://api.github.com/users/jmschrei/received_events","type":"User","site_admin":false},"repo":{"id":37904379,"name":"scikit-learn","full_name":"jmschrei/scikit-learn","owner":{"login":"jmschrei","id":3916816,"avatar_url":"https://avatars.githubusercontent.com/u/3916816?v=3","gravatar_id":"","url":"https://api.github.com/users/jmschrei","html_url":"https://github.com/jmschrei","followers_url":"https://api.github.com/users/jmschrei/followers","following_url":"https://api.github.com/users/jmschrei/following{/other_user}","gists_url":"https://api.github.com/users/jmschrei/gists{/gist_id}","starred_url":"https://api.github.com/users/jmschrei/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jmschrei/subscriptions","organizations_url":"https://api.github.com/users/jmschrei/orgs","repos_url":"https://api.github.com/users/jmschrei/repos","events_url":"https://api.github.com/users/jmschrei/events{/privacy}","received_events_url":"https://api.github.com/users/jmschrei/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/jmschrei/scikit-learn","description":"scikit-learn: machine learning in Python","fork":true,"url":"https://api.github.com/repos/jmschrei/scikit-learn","forks_url":"https://api.github.com/repos/jmschrei/scikit-learn/forks","keys_url":"https://api.github.com/repos/jmschrei/scikit-learn/keys{/key_id}","collaborators_url":"https://api.github.com/repos/jmschrei/scikit-learn/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/jmschrei/scikit-learn/teams","hooks_url":"https://api.github.com/repos/jmschrei/scikit-learn/hooks","issue_events_url":"https://api.github.com/repos/jmschrei/scikit-learn/issues/events{/number}","events_url":"https://api.github.com/repos/jmschrei/scikit-learn/events","assignees_url":"https://api.github.com/repos/jmschrei/scikit-learn/assignees{/user}","branches_url":"https://api.github.com/repos/jmschrei/scikit-learn/branches{/branch}","tags_url":"https://api.github.com/repos/jmschrei/scikit-learn/tags","blobs_url":"https://api.github.com/repos/jmschrei/scikit-learn/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/jmschrei/scikit-learn/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/jmschrei/scikit-learn/git/refs{/sha}","trees_url":"https://api.github.com/repos/jmschrei/scikit-learn/git/trees{/sha}","statuses_url":"https://api.github.com/repos/jmschrei/scikit-learn/statuses/{sha}","languages_url":"https://api.github.com/repos/jmschrei/scikit-learn/languages","stargazers_url":"https://api.github.com/repos/jmschrei/scikit-learn/stargazers","contributors_url":"https://api.github.com/repos/jmschrei/scikit-learn/contributors","subscribers_url":"https://api.github.com/repos/jmschrei/scikit-learn/subscribers","subscription_url":"https://api.github.com/repos/jmschrei/scikit-learn/subscription","commits_url":"https://api.github.com/repos/jmschrei/scikit-learn/commits{/sha}","git_commits_url":"https://api.github.com/repos/jmschrei/scikit-learn/git/commits{/sha}","comments_url":"https://api.github.com/repos/jmschrei/scikit-learn/comments{/number}","issue_comment_url":"https://api.github.com/repos/jmschrei/scikit-learn/issues/comments{/number}","contents_url":"https://api.github.com/repos/jmschrei/scikit-learn/contents/{+path}","compare_url":"https://api.github.com/repos/jmschrei/scikit-learn/compare/{base}...{head}","merges_url":"https://api.github.com/repos/jmschrei/scikit-learn/merges","archive_url":"https://api.github.com/repos/jmschrei/scikit-learn/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/jmschrei/scikit-learn/downloads","issues_url":"https://api.github.com/repos/jmschrei/scikit-learn/issues{/number}","pulls_url":"https://api.github.com/repos/jmschrei/scikit-learn/pulls{/number}","milestones_url":"https://api.github.com/repos/jmschrei/scikit-learn/milestones{/number}","notifications_url":"https://api.github.com/repos/jmschrei/scikit-learn/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/jmschrei/scikit-learn/labels{/name}","releases_url":"https://api.github.com/repos/jmschrei/scikit-learn/releases{/id}","created_at":"2015-06-23T07:38:19Z","updated_at":"2015-06-23T07:38:27Z","pushed_at":"2015-09-16T13:57:21Z","git_url":"git://github.com/jmschrei/scikit-learn.git","ssh_url":"git@github.com:jmschrei/scikit-learn.git","clone_url":"https://github.com/jmschrei/scikit-learn.git","svn_url":"https://github.com/jmschrei/scikit-learn","homepage":"http://scikit-learn.org","size":71124,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"scikit-learn:master","ref":"master","sha":"ebd5dbc68a6645e0cb024742b20fc2c772357028","user":{"login":"scikit-learn","id":365630,"avatar_url":"https://avatars.githubusercontent.com/u/365630?v=3","gravatar_id":"","url":"https://api.github.com/users/scikit-learn","html_url":"https://github.com/scikit-learn","followers_url":"https://api.github.com/users/scikit-learn/followers","following_url":"https://api.github.com/users/scikit-learn/following{/other_user}","gists_url":"https://api.github.com/users/scikit-learn/gists{/gist_id}","starred_url":"https://api.github.com/users/scikit-learn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/scikit-learn/subscriptions","organizations_url":"https://api.github.com/users/scikit-learn/orgs","repos_url":"https://api.github.com/users/scikit-learn/repos","events_url":"https://api.github.com/users/scikit-learn/events{/privacy}","received_events_url":"https://api.github.com/users/scikit-learn/received_events","type":"Organization","site_admin":false},"repo":{"id":843222,"name":"scikit-learn","full_name":"scikit-learn/scikit-learn","owner":{"login":"scikit-learn","id":365630,"avatar_url":"https://avatars.githubusercontent.com/u/365630?v=3","gravatar_id":"","url":"https://api.github.com/users/scikit-learn","html_url":"https://github.com/scikit-learn","followers_url":"https://api.github.com/users/scikit-learn/followers","following_url":"https://api.github.com/users/scikit-learn/following{/other_user}","gists_url":"https://api.github.com/users/scikit-learn/gists{/gist_id}","starred_url":"https://api.github.com/users/scikit-learn/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/scikit-learn/subscriptions","organizations_url":"https://api.github.com/users/scikit-learn/orgs","repos_url":"https://api.github.com/users/scikit-learn/repos","events_url":"https://api.github.com/users/scikit-learn/events{/privacy}","received_events_url":"https://api.github.com/users/scikit-learn/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/scikit-learn/scikit-learn","description":"scikit-learn: machine learning in Python","fork":false,"url":"https://api.github.com/repos/scikit-learn/scikit-learn","forks_url":"https://api.github.com/repos/scikit-learn/scikit-learn/forks","keys_url":"https://api.github.com/repos/scikit-learn/scikit-learn/keys{/key_id}","collaborators_url":"https://api.github.com/repos/scikit-learn/scikit-learn/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/scikit-learn/scikit-learn/teams","hooks_url":"https://api.github.com/repos/scikit-learn/scikit-learn/hooks","issue_events_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/events{/number}","events_url":"https://api.github.com/repos/scikit-learn/scikit-learn/events","assignees_url":"https://api.github.com/repos/scikit-learn/scikit-learn/assignees{/user}","branches_url":"https://api.github.com/repos/scikit-learn/scikit-learn/branches{/branch}","tags_url":"https://api.github.com/repos/scikit-learn/scikit-learn/tags","blobs_url":"https://api.github.com/repos/scikit-learn/scikit-learn/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/scikit-learn/scikit-learn/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/scikit-learn/scikit-learn/git/refs{/sha}","trees_url":"https://api.github.com/repos/scikit-learn/scikit-learn/git/trees{/sha}","statuses_url":"https://api.github.com/repos/scikit-learn/scikit-learn/statuses/{sha}","languages_url":"https://api.github.com/repos/scikit-learn/scikit-learn/languages","stargazers_url":"https://api.github.com/repos/scikit-learn/scikit-learn/stargazers","contributors_url":"https://api.github.com/repos/scikit-learn/scikit-learn/contributors","subscribers_url":"https://api.github.com/repos/scikit-learn/scikit-learn/subscribers","subscription_url":"https://api.github.com/repos/scikit-learn/scikit-learn/subscription","commits_url":"https://api.github.com/repos/scikit-learn/scikit-learn/commits{/sha}","git_commits_url":"https://api.github.com/repos/scikit-learn/scikit-learn/git/commits{/sha}","comments_url":"https://api.github.com/repos/scikit-learn/scikit-learn/comments{/number}","issue_comment_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/comments{/number}","contents_url":"https://api.github.com/repos/scikit-learn/scikit-learn/contents/{+path}","compare_url":"https://api.github.com/repos/scikit-learn/scikit-learn/compare/{base}...{head}","merges_url":"https://api.github.com/repos/scikit-learn/scikit-learn/merges","archive_url":"https://api.github.com/repos/scikit-learn/scikit-learn/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/scikit-learn/scikit-learn/downloads","issues_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues{/number}","pulls_url":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls{/number}","milestones_url":"https://api.github.com/repos/scikit-learn/scikit-learn/milestones{/number}","notifications_url":"https://api.github.com/repos/scikit-learn/scikit-learn/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/scikit-learn/scikit-learn/labels{/name}","releases_url":"https://api.github.com/repos/scikit-learn/scikit-learn/releases{/id}","created_at":"2010-08-17T09:43:38Z","updated_at":"2015-09-17T03:19:05Z","pushed_at":"2015-09-17T06:04:34Z","git_url":"git://github.com/scikit-learn/scikit-learn.git","ssh_url":"git@github.com:scikit-learn/scikit-learn.git","clone_url":"https://github.com/scikit-learn/scikit-learn.git","svn_url":"https://github.com/scikit-learn/scikit-learn","homepage":"http://scikit-learn.org","size":133192,"stargazers_count":7520,"watchers_count":7520,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":4449,"mirror_url":null,"open_issues_count":754,"forks":4449,"open_issues":754,"watchers":7520,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/5278"},"html":{"href":"https://github.com/scikit-learn/scikit-learn/pull/5278"},"issue":{"href":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/5278"},"comments":{"href":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/5278/comments"},"review_comments":{"href":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/5278/comments"},"review_comment":{"href":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/5278/commits"},"statuses":{"href":"https://api.github.com/repos/scikit-learn/scikit-learn/statuses/201eb23b119ad0a7e687a3f7d3204be09af8d1f6"}}}}