{"action":"created","issue":{"url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/5296","repository_url":"https://api.github.com/repos/scikit-learn/scikit-learn","labels_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/5296/labels{/name}","comments_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/5296/comments","events_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/5296/events","html_url":"https://github.com/scikit-learn/scikit-learn/pull/5296","id":107757853,"number":5296,"title":"[MRG] Add scaling to alpha regularization parameter in NMF","user":{"login":"TomDLT","id":11065596,"avatar_url":"https://avatars.githubusercontent.com/u/11065596?v=3","gravatar_id":"","url":"https://api.github.com/users/TomDLT","html_url":"https://github.com/TomDLT","followers_url":"https://api.github.com/users/TomDLT/followers","following_url":"https://api.github.com/users/TomDLT/following{/other_user}","gists_url":"https://api.github.com/users/TomDLT/gists{/gist_id}","starred_url":"https://api.github.com/users/TomDLT/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/TomDLT/subscriptions","organizations_url":"https://api.github.com/users/TomDLT/orgs","repos_url":"https://api.github.com/users/TomDLT/repos","events_url":"https://api.github.com/users/TomDLT/events{/privacy}","received_events_url":"https://api.github.com/users/TomDLT/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/scikit-learn/scikit-learn/labels/Needs%20Review","name":"Needs Review","color":"f7c6c7"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":2,"created_at":"2015-09-22T17:06:03Z","updated_at":"2016-10-05T13:44:48Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/5296","html_url":"https://github.com/scikit-learn/scikit-learn/pull/5296","diff_url":"https://github.com/scikit-learn/scikit-learn/pull/5296.diff","patch_url":"https://github.com/scikit-learn/scikit-learn/pull/5296.patch"},"body":"(I separated this modification from https://github.com/scikit-learn/scikit-learn/pull/4852#issuecomment-132255591 for further discussion about it.)\r\nIn NMF, I scaled here the regularization parameter `alpha` with `n_samples` and `n_features`.\r\n___\r\n Indeed, without scaling, two problems appear:\r\n- The regularization penalizes the sum of coefficients in W and H. If the number of element in W and H is not the same (i.e. `n_samples != n_features`), the constraint is unbalanced. One of H or W goes to zero, and the other one, which is less penalized, increases to compensate.\r\n- The value of alpha that makes the coefficients of W and H collapse is proportional with `sqrt(n_features * n_samples)`. It makes the scaling of alpha depends on the size of the data.\r\n\r\n**Test to prove the point:** I tested several sizes for the input `X`, and plotted how the coefficients in W and H collapse with respect to the `alpha` parameter.\r\n\r\nwithout scaling alpha:\r\n![noregul](https://cloud.githubusercontent.com/assets/11065596/10025176/31ef9e1a-615b-11e5-86b5-70f42ec45442.png)\r\nwith properly scaling alpha:\r\n![regul](https://cloud.githubusercontent.com/assets/11065596/10025177/31f14080-615b-11e5-90b3-a26a751ce7a6.png)\r\n\r\n**Scaling used:** I used `alpha_W = alpha * n_features` and `alpha_H = alpha * n_samples`.\r\n\r\n**Conclusion:** the effect of the `alpha` parameter is much more consistent if we scale it.\r\n___\r\nAs L1 and L2 regularizations in NMF is fresh new (#4852), it would not really break any code (before 0.17 at least).\r\nBut do we want to add this? \r\nIs it consistent with other estimators in scikit-learn?\r\nWhat Do You Think?\r\n\r\n@vene @mblondel "},"comment":{"url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/comments/251678638","html_url":"https://github.com/scikit-learn/scikit-learn/pull/5296#issuecomment-251678638","issue_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/5296","id":251678638,"user":{"login":"TomDLT","id":11065596,"avatar_url":"https://avatars.githubusercontent.com/u/11065596?v=3","gravatar_id":"","url":"https://api.github.com/users/TomDLT","html_url":"https://github.com/TomDLT","followers_url":"https://api.github.com/users/TomDLT/followers","following_url":"https://api.github.com/users/TomDLT/following{/other_user}","gists_url":"https://api.github.com/users/TomDLT/gists{/gist_id}","starred_url":"https://api.github.com/users/TomDLT/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/TomDLT/subscriptions","organizations_url":"https://api.github.com/users/TomDLT/orgs","repos_url":"https://api.github.com/users/TomDLT/repos","events_url":"https://api.github.com/users/TomDLT/events{/privacy}","received_events_url":"https://api.github.com/users/TomDLT/received_events","type":"User","site_admin":false},"created_at":"2016-10-05T13:44:48Z","updated_at":"2016-10-05T13:44:48Z","body":"The plots show the mean of all elements in W and H, i.e. the L1 norm normalized by the number of elements.\r\n\r\nIt all came from @vene's comment (https://github.com/scikit-learn/scikit-learn/pull/4852#issuecomment-132255591), and to avoid blocking the PR, I opened a separate PR to further talk about it.\r\nI have never seen any such scaling in the literature, yet as @vene's puts it (https://github.com/scikit-learn/scikit-learn/pull/4852#issuecomment-138350912), \"it's a usability detail, [...] But usability is important\". I guess this is the difference between a paper's implementation and a general public package's implementation. At the same time, I understand the need to avoid non-standard behaviors.\r\n\r\n"}}
{"actor":{"display_login":"TomDLT"}}