{"action":"created","comment":{"url":"https://api.github.com/repos/mlpack/mlpack/pulls/comments/165500819","pull_request_review_id":93453254,"id":165500819,"diff_hunk":"@@ -0,0 +1,56 @@\n+/**\n+ * @file delete_allocated_memory.hpp","path":"src/mlpack/bindings/tests/delete_allocated_memory.hpp","position":2,"original_position":2,"commit_id":"ac89b49e27e7f3df8c23cd92249ff6a44a4be7ef","original_commit_id":"a402f61455ff1a0f2fd2eab10b06d997e051e395","user":{"login":"zoq","id":4209744,"avatar_url":"https://avatars3.githubusercontent.com/u/4209744?v=4","gravatar_id":"","url":"https://api.github.com/users/zoq","html_url":"https://github.com/zoq","followers_url":"https://api.github.com/users/zoq/followers","following_url":"https://api.github.com/users/zoq/following{/other_user}","gists_url":"https://api.github.com/users/zoq/gists{/gist_id}","starred_url":"https://api.github.com/users/zoq/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/zoq/subscriptions","organizations_url":"https://api.github.com/users/zoq/orgs","repos_url":"https://api.github.com/users/zoq/repos","events_url":"https://api.github.com/users/zoq/events{/privacy}","received_events_url":"https://api.github.com/users/zoq/received_events","type":"User","site_admin":false},"body":"I see, thanks for the clarification.","created_at":"2018-02-01T21:58:32Z","updated_at":"2018-02-01T21:58:33Z","html_url":"https://github.com/mlpack/mlpack/pull/1214#discussion_r165500819","pull_request_url":"https://api.github.com/repos/mlpack/mlpack/pulls/1214","author_association":"OWNER","_links":{"self":{"href":"https://api.github.com/repos/mlpack/mlpack/pulls/comments/165500819"},"html":{"href":"https://github.com/mlpack/mlpack/pull/1214#discussion_r165500819"},"pull_request":{"href":"https://api.github.com/repos/mlpack/mlpack/pulls/1214"}},"in_reply_to_id":164891151},"pull_request":{"url":"https://api.github.com/repos/mlpack/mlpack/pulls/1214","id":164993896,"html_url":"https://github.com/mlpack/mlpack/pull/1214","diff_url":"https://github.com/mlpack/mlpack/pull/1214.diff","patch_url":"https://github.com/mlpack/mlpack/pull/1214.patch","issue_url":"https://api.github.com/repos/mlpack/mlpack/issues/1214","number":1214,"state":"open","locked":false,"title":"Python memory fix","user":{"login":"rcurtin","id":1845039,"avatar_url":"https://avatars0.githubusercontent.com/u/1845039?v=4","gravatar_id":"","url":"https://api.github.com/users/rcurtin","html_url":"https://github.com/rcurtin","followers_url":"https://api.github.com/users/rcurtin/followers","following_url":"https://api.github.com/users/rcurtin/following{/other_user}","gists_url":"https://api.github.com/users/rcurtin/gists{/gist_id}","starred_url":"https://api.github.com/users/rcurtin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rcurtin/subscriptions","organizations_url":"https://api.github.com/users/rcurtin/orgs","repos_url":"https://api.github.com/users/rcurtin/repos","events_url":"https://api.github.com/users/rcurtin/events{/privacy}","received_events_url":"https://api.github.com/users/rcurtin/received_events","type":"User","site_admin":false},"body":"This is a set of bugfixes for the Python bindings that improve several issues including fixing #1201.  @nvasil: you can apply the patches from here or use this branch and it will fix your memory issue---I tested with the script from #1201.\r\n\r\nA more clear summary of changes:\r\n\r\n * Avoid copies of matrices and other large types by using `std::move()` when calling `CLI::SetParam<>()` from the generated Cython bindings.\r\n\r\n * Clarify and correct ownership of matrix memory: now, in all cases, matrices passed down from Python will retain their ownership of the memory, and internally in mlpack we will use non-strict aliases so that we never delete the user's memory.\r\n\r\n * As a result of the code above, any binding that uses `std::move()` on the input matrix (most of them do) will now result in the Python user's matrix *not* being empty.\r\n\r\n * Also we print default values for optional string/double/int parameters now, which is nice.\r\n\r\nThere is more to do before I merge this, though:\r\n\r\n- [x] Handle the model ownership issue: right now an input model gets wiped out and the user has to use `copy.copy()`.  Instead we should have `CLI` hold pointers to models instead, which allows us to make `output_model` point to the same model as `input_model`.\r\n\r\n- [x] Force copies by default but provide a clear warning that extra copies are being done.  We can add an option `make_copies` and if `make_copies=False`, then we use aliases everywhere but the user's input matrices or models may be modified.  I'm not sure if I like defaulting `make_copies` to `True` or `False`; there are advantages and disadvantages to each.  I'm slightly inclined to set `make_copies=False` by default, then make clear in the documentation that if \"weird things\" are happening, try `make_copies=True`.\r\n\r\nIn any case, as it stands this fixes #1201.","created_at":"2018-01-24T23:42:28Z","updated_at":"2018-02-01T21:58:33Z","closed_at":null,"merged_at":null,"merge_commit_sha":"1ca5dce0221f02f75639332e133e0a813deba1e7","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"milestone":null,"commits_url":"https://api.github.com/repos/mlpack/mlpack/pulls/1214/commits","review_comments_url":"https://api.github.com/repos/mlpack/mlpack/pulls/1214/comments","review_comment_url":"https://api.github.com/repos/mlpack/mlpack/pulls/comments{/number}","comments_url":"https://api.github.com/repos/mlpack/mlpack/issues/1214/comments","statuses_url":"https://api.github.com/repos/mlpack/mlpack/statuses/ac89b49e27e7f3df8c23cd92249ff6a44a4be7ef","head":{"label":"rcurtin:python-memfix","ref":"python-memfix","sha":"ac89b49e27e7f3df8c23cd92249ff6a44a4be7ef","user":{"login":"rcurtin","id":1845039,"avatar_url":"https://avatars0.githubusercontent.com/u/1845039?v=4","gravatar_id":"","url":"https://api.github.com/users/rcurtin","html_url":"https://github.com/rcurtin","followers_url":"https://api.github.com/users/rcurtin/followers","following_url":"https://api.github.com/users/rcurtin/following{/other_user}","gists_url":"https://api.github.com/users/rcurtin/gists{/gist_id}","starred_url":"https://api.github.com/users/rcurtin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rcurtin/subscriptions","organizations_url":"https://api.github.com/users/rcurtin/orgs","repos_url":"https://api.github.com/users/rcurtin/repos","events_url":"https://api.github.com/users/rcurtin/events{/privacy}","received_events_url":"https://api.github.com/users/rcurtin/received_events","type":"User","site_admin":false},"repo":{"id":28698514,"name":"mlpack","full_name":"rcurtin/mlpack","owner":{"login":"rcurtin","id":1845039,"avatar_url":"https://avatars0.githubusercontent.com/u/1845039?v=4","gravatar_id":"","url":"https://api.github.com/users/rcurtin","html_url":"https://github.com/rcurtin","followers_url":"https://api.github.com/users/rcurtin/followers","following_url":"https://api.github.com/users/rcurtin/following{/other_user}","gists_url":"https://api.github.com/users/rcurtin/gists{/gist_id}","starred_url":"https://api.github.com/users/rcurtin/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/rcurtin/subscriptions","organizations_url":"https://api.github.com/users/rcurtin/orgs","repos_url":"https://api.github.com/users/rcurtin/repos","events_url":"https://api.github.com/users/rcurtin/events{/privacy}","received_events_url":"https://api.github.com/users/rcurtin/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/rcurtin/mlpack","description":"mlpack: a scalable C++ machine learning library","fork":true,"url":"https://api.github.com/repos/rcurtin/mlpack","forks_url":"https://api.github.com/repos/rcurtin/mlpack/forks","keys_url":"https://api.github.com/repos/rcurtin/mlpack/keys{/key_id}","collaborators_url":"https://api.github.com/repos/rcurtin/mlpack/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/rcurtin/mlpack/teams","hooks_url":"https://api.github.com/repos/rcurtin/mlpack/hooks","issue_events_url":"https://api.github.com/repos/rcurtin/mlpack/issues/events{/number}","events_url":"https://api.github.com/repos/rcurtin/mlpack/events","assignees_url":"https://api.github.com/repos/rcurtin/mlpack/assignees{/user}","branches_url":"https://api.github.com/repos/rcurtin/mlpack/branches{/branch}","tags_url":"https://api.github.com/repos/rcurtin/mlpack/tags","blobs_url":"https://api.github.com/repos/rcurtin/mlpack/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/rcurtin/mlpack/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/rcurtin/mlpack/git/refs{/sha}","trees_url":"https://api.github.com/repos/rcurtin/mlpack/git/trees{/sha}","statuses_url":"https://api.github.com/repos/rcurtin/mlpack/statuses/{sha}","languages_url":"https://api.github.com/repos/rcurtin/mlpack/languages","stargazers_url":"https://api.github.com/repos/rcurtin/mlpack/stargazers","contributors_url":"https://api.github.com/repos/rcurtin/mlpack/contributors","subscribers_url":"https://api.github.com/repos/rcurtin/mlpack/subscribers","subscription_url":"https://api.github.com/repos/rcurtin/mlpack/subscription","commits_url":"https://api.github.com/repos/rcurtin/mlpack/commits{/sha}","git_commits_url":"https://api.github.com/repos/rcurtin/mlpack/git/commits{/sha}","comments_url":"https://api.github.com/repos/rcurtin/mlpack/comments{/number}","issue_comment_url":"https://api.github.com/repos/rcurtin/mlpack/issues/comments{/number}","contents_url":"https://api.github.com/repos/rcurtin/mlpack/contents/{+path}","compare_url":"https://api.github.com/repos/rcurtin/mlpack/compare/{base}...{head}","merges_url":"https://api.github.com/repos/rcurtin/mlpack/merges","archive_url":"https://api.github.com/repos/rcurtin/mlpack/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/rcurtin/mlpack/downloads","issues_url":"https://api.github.com/repos/rcurtin/mlpack/issues{/number}","pulls_url":"https://api.github.com/repos/rcurtin/mlpack/pulls{/number}","milestones_url":"https://api.github.com/repos/rcurtin/mlpack/milestones{/number}","notifications_url":"https://api.github.com/repos/rcurtin/mlpack/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/rcurtin/mlpack/labels{/name}","releases_url":"https://api.github.com/repos/rcurtin/mlpack/releases{/id}","deployments_url":"https://api.github.com/repos/rcurtin/mlpack/deployments","created_at":"2015-01-01T23:37:11Z","updated_at":"2016-11-29T14:34:54Z","pushed_at":"2018-02-01T21:27:35Z","git_url":"git://github.com/rcurtin/mlpack.git","ssh_url":"git@github.com:rcurtin/mlpack.git","clone_url":"https://github.com/rcurtin/mlpack.git","svn_url":"https://github.com/rcurtin/mlpack","homepage":null,"size":240367,"stargazers_count":5,"watchers_count":5,"language":"C++","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":1,"mirror_url":null,"archived":false,"open_issues_count":0,"license":null,"forks":1,"open_issues":0,"watchers":5,"default_branch":"master"}},"base":{"label":"mlpack:master","ref":"master","sha":"73c576e20821267a284b48e4bdac61b119411c48","user":{"login":"mlpack","id":10216045,"avatar_url":"https://avatars0.githubusercontent.com/u/10216045?v=4","gravatar_id":"","url":"https://api.github.com/users/mlpack","html_url":"https://github.com/mlpack","followers_url":"https://api.github.com/users/mlpack/followers","following_url":"https://api.github.com/users/mlpack/following{/other_user}","gists_url":"https://api.github.com/users/mlpack/gists{/gist_id}","starred_url":"https://api.github.com/users/mlpack/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mlpack/subscriptions","organizations_url":"https://api.github.com/users/mlpack/orgs","repos_url":"https://api.github.com/users/mlpack/repos","events_url":"https://api.github.com/users/mlpack/events{/privacy}","received_events_url":"https://api.github.com/users/mlpack/received_events","type":"Organization","site_admin":false},"repo":{"id":28149409,"name":"mlpack","full_name":"mlpack/mlpack","owner":{"login":"mlpack","id":10216045,"avatar_url":"https://avatars0.githubusercontent.com/u/10216045?v=4","gravatar_id":"","url":"https://api.github.com/users/mlpack","html_url":"https://github.com/mlpack","followers_url":"https://api.github.com/users/mlpack/followers","following_url":"https://api.github.com/users/mlpack/following{/other_user}","gists_url":"https://api.github.com/users/mlpack/gists{/gist_id}","starred_url":"https://api.github.com/users/mlpack/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/mlpack/subscriptions","organizations_url":"https://api.github.com/users/mlpack/orgs","repos_url":"https://api.github.com/users/mlpack/repos","events_url":"https://api.github.com/users/mlpack/events{/privacy}","received_events_url":"https://api.github.com/users/mlpack/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/mlpack/mlpack","description":"mlpack: a scalable C++ machine learning library -- ","fork":false,"url":"https://api.github.com/repos/mlpack/mlpack","forks_url":"https://api.github.com/repos/mlpack/mlpack/forks","keys_url":"https://api.github.com/repos/mlpack/mlpack/keys{/key_id}","collaborators_url":"https://api.github.com/repos/mlpack/mlpack/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/mlpack/mlpack/teams","hooks_url":"https://api.github.com/repos/mlpack/mlpack/hooks","issue_events_url":"https://api.github.com/repos/mlpack/mlpack/issues/events{/number}","events_url":"https://api.github.com/repos/mlpack/mlpack/events","assignees_url":"https://api.github.com/repos/mlpack/mlpack/assignees{/user}","branches_url":"https://api.github.com/repos/mlpack/mlpack/branches{/branch}","tags_url":"https://api.github.com/repos/mlpack/mlpack/tags","blobs_url":"https://api.github.com/repos/mlpack/mlpack/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/mlpack/mlpack/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/mlpack/mlpack/git/refs{/sha}","trees_url":"https://api.github.com/repos/mlpack/mlpack/git/trees{/sha}","statuses_url":"https://api.github.com/repos/mlpack/mlpack/statuses/{sha}","languages_url":"https://api.github.com/repos/mlpack/mlpack/languages","stargazers_url":"https://api.github.com/repos/mlpack/mlpack/stargazers","contributors_url":"https://api.github.com/repos/mlpack/mlpack/contributors","subscribers_url":"https://api.github.com/repos/mlpack/mlpack/subscribers","subscription_url":"https://api.github.com/repos/mlpack/mlpack/subscription","commits_url":"https://api.github.com/repos/mlpack/mlpack/commits{/sha}","git_commits_url":"https://api.github.com/repos/mlpack/mlpack/git/commits{/sha}","comments_url":"https://api.github.com/repos/mlpack/mlpack/comments{/number}","issue_comment_url":"https://api.github.com/repos/mlpack/mlpack/issues/comments{/number}","contents_url":"https://api.github.com/repos/mlpack/mlpack/contents/{+path}","compare_url":"https://api.github.com/repos/mlpack/mlpack/compare/{base}...{head}","merges_url":"https://api.github.com/repos/mlpack/mlpack/merges","archive_url":"https://api.github.com/repos/mlpack/mlpack/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/mlpack/mlpack/downloads","issues_url":"https://api.github.com/repos/mlpack/mlpack/issues{/number}","pulls_url":"https://api.github.com/repos/mlpack/mlpack/pulls{/number}","milestones_url":"https://api.github.com/repos/mlpack/mlpack/milestones{/number}","notifications_url":"https://api.github.com/repos/mlpack/mlpack/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/mlpack/mlpack/labels{/name}","releases_url":"https://api.github.com/repos/mlpack/mlpack/releases{/id}","deployments_url":"https://api.github.com/repos/mlpack/mlpack/deployments","created_at":"2014-12-17T18:16:59Z","updated_at":"2018-02-01T16:35:05Z","pushed_at":"2018-02-01T21:56:26Z","git_url":"git://github.com/mlpack/mlpack.git","ssh_url":"git@github.com:mlpack/mlpack.git","clone_url":"https://github.com/mlpack/mlpack.git","svn_url":"https://github.com/mlpack/mlpack","homepage":"http://www.mlpack.org/","size":242013,"stargazers_count":1906,"watchers_count":1906,"language":"C++","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":694,"mirror_url":null,"archived":false,"open_issues_count":109,"license":null,"forks":694,"open_issues":109,"watchers":1906,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/mlpack/mlpack/pulls/1214"},"html":{"href":"https://github.com/mlpack/mlpack/pull/1214"},"issue":{"href":"https://api.github.com/repos/mlpack/mlpack/issues/1214"},"comments":{"href":"https://api.github.com/repos/mlpack/mlpack/issues/1214/comments"},"review_comments":{"href":"https://api.github.com/repos/mlpack/mlpack/pulls/1214/comments"},"review_comment":{"href":"https://api.github.com/repos/mlpack/mlpack/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/mlpack/mlpack/pulls/1214/commits"},"statuses":{"href":"https://api.github.com/repos/mlpack/mlpack/statuses/ac89b49e27e7f3df8c23cd92249ff6a44a4be7ef"}},"author_association":"OWNER"}}
{"actor":{"display_login":"zoq"}}