You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by jojo19893 <gi...@git.apache.org> on 2015/05/07 12:18:27 UTC
[GitHub] flink pull request: Lossfunctions
GitHub user jojo19893 opened a pull request:
https://github.com/apache/flink/pull/656
Lossfunctions
We added Logistic Loss Functions and Hinge Loss to the Optimazation Framework.
See for the implemented Functions:
https://github.com/JohnLangford/vowpal_wabbit/wiki/Loss-functions
Jira Issue:
https://issues.apache.org/jira/browse/FLINK-1979
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jojo19893/flink master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/flink/pull/656.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #656
----
commit 4431e1d2ed0ebfd230ae997bcbf412c965108034
Author: Theodore Vasiloudis <tv...@sics.se>
Date: 2015-04-21T08:59:34Z
[FLINK-1807] [ml] Adds optimization framework and SGD solver.
Added Stochastic Gradient Descent initial version and some tests.
Added L1, L2 regularization.
Added tests for regularization, fixed parameter setting.
commit e51c63583514d51f727816944df01a0e6e8461eb
Author: Theodore Vasiloudis <tv...@sics.se>
Date: 2015-04-27T14:16:18Z
Added documentation, some minor fixes.
commit 4a2235c4606b03fbee8854dd02dea7faa27ace9b
Author: Theodore Vasiloudis <tv...@sics.se>
Date: 2015-04-28T13:43:17Z
Added license to doc file
commit afb281c0273af944fabdca96d833d95a094e7944
Author: Theodore Vasiloudis <tv...@sics.se>
Date: 2015-04-29T12:36:12Z
Style fixes
commit 9a810b70f55fc62164d873678437f995b44f4d8e
Author: Theodore Vasiloudis <tv...@sics.se>
Date: 2015-05-04T08:50:49Z
Refactored the way regularization is applied.
We are now using pattern matching to determine if a regularization type is
differentiable or not. If it is (L2) we apply the regularization at the gradient
calculation step, before taking the update step. If it isn't (L1) the regularization
is applied after the gradient descent step has been taken. This sets us up nicely
for the L-BFGS algorithm, where we can calculate the regularized loss and gradient
required if we are using L2.
commit 9c71e1a18f4011fdaec53945308c230ab6a97752
Author: Theodore Vasiloudis <tv...@sics.se>
Date: 2015-05-05T08:50:52Z
Added option to provide UDF for the prediction function, moved SGD regularization to update step.
Incorporated the rest of Till's comments.
commit 3a0ef8588290c17e83bc0ffa86f1e54d10bf39e0
Author: Theodore Vasiloudis <tv...@sics.se>
Date: 2015-05-05T12:16:50Z
Style hotfix
commit 8314594d547557886630f3076c8f0a72bb478fac
Author: Theodore Vasiloudis <tv...@sics.se>
Date: 2015-05-05T12:35:53Z
Regularization test check fix
commit b8ec680d7833669e19f46c6c69f29b76b82d18f5
Author: Theodore Vasiloudis <tv...@sics.se>
Date: 2015-05-06T14:34:08Z
Added prediction function class to alow non-linear optimization in the future.
Small refactoring to allow calculation of regularized loss separatly from
regularized gradient.
commit 63115fbeff237642e1be87f143cb5042a4aeeff7
Author: Johannes Günther <jguenth1>
Date: 2015-05-07T09:46:24Z
Implemented Hinge Loss
commit aca48e33b4cb9c90006a77caddc5fa8f8c057217
Author: mguldner <ma...@gmail.com>
Date: 2015-05-07T09:49:12Z
Add LogisticLoss Function
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1979] Lossfunctions
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on the pull request:
https://github.com/apache/flink/pull/656#issuecomment-100884364
Thank you Johaness. The optimization code has been merged to the master now, so could you rebase your branch to the latest master so we can look at the changes in an isolated way?
You can take a look at the [How to contibute](http://flink.apache.org/how-to-contribute.html#contributing-code-&-documentation) guide on how to do this. The merges you have currently make it hard to review the code.
Also, please make sure all your classes have docstrings, you can take the docstring for SquaredLoss as an example (i.e. one sentence is usually enough).
Documentation is always welcome of course, so if you want to add some more details to the loss functions section of the ML documentation (docs/libs/ml/optimization.md) feel free to do so in this PR.
Let me know if you run into any problems.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1979] Lossfunctions
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on the pull request:
https://github.com/apache/flink/pull/656#issuecomment-218399619
Hello @skavulya thank you for your contribution, I'm looking forward to the PR!
Re. the regularization penalty, we it changed to make user choice easier, e.g. preventing the use of an L1 penalty when L-BFGS is used.
Personally I would prefer to go back to the original design and have it as a settable parameter, and apply some sanity checking within the optimizer to avoid cases like the one mentioned above.
I would suggest you open the PR and we can discuss it there.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1979] Lossfunctions
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on the pull request:
https://github.com/apache/flink/pull/656#issuecomment-113504786
Hello, this PR needs to be rebase since so that it's mergeable with the latest master.
I think it's fair to say that we can close it in a week if no more activity occurs.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1979] Lossfunctions
Posted by skavulya <gi...@git.apache.org>.
Github user skavulya commented on the pull request:
https://github.com/apache/flink/pull/656#issuecomment-209618076
I need this pull request for work I am doing with L-BFGS. I would like to take it over and make the recommended changes since there has not been any activity on this for a while.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1979] Lossfunctions
Posted by jojo19893 <gi...@git.apache.org>.
Github user jojo19893 commented on the pull request:
https://github.com/apache/flink/pull/656#issuecomment-100696661
Okay I Changed the implementation of the logistic loss! Thanks for the advice !
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1979] Lossfunctions
Posted by skavulya <gi...@git.apache.org>.
Github user skavulya commented on the pull request:
https://github.com/apache/flink/pull/656#issuecomment-218651924
thanks @thvasilo! I created the PR https://github.com/apache/flink/pull/1985
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1979] Lossfunctions
Posted by thvasilo <gi...@git.apache.org>.
Github user thvasilo commented on the pull request:
https://github.com/apache/flink/pull/656#issuecomment-100159626
Thank you for the contribution Johaness. For the logistic loss could you change the implementation to match the approximate version that sklearn (and other libraries) use?
You can see it [here](https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/linear_model/sgd_fast.pyx#L202), this way we may save the computation of the log.
@tillrohrmann Any objections to using this version?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1979] Lossfunctions
Posted by skavulya <gi...@git.apache.org>.
Github user skavulya commented on the pull request:
https://github.com/apache/flink/pull/656#issuecomment-218380988
@tillrohrmann I updated the loss functions and regularization penalties based on your branch. I was not sure whether to update the gradient descent algorithm to pass the regularization penalty as a parameter in this branch so I kept the GradientDescent API as-is.
Let me know if you would like me to change the GradientDescent API to GradientDescent().setRegularizationPenalty(L1Regularization) and I will update my branch, squash the commits, and create a PR. https://github.com/apache/flink/compare/master...skavulya:loss-functions
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1979] Lossfunctions
Posted by aalexandrov <gi...@git.apache.org>.
Github user aalexandrov commented on the pull request:
https://github.com/apache/flink/pull/656#issuecomment-106907665
This needs a PR before it is merged. @jojo19893, @mguldner please cherry pick your changes such that you have only one or two commits and force push in this branch as suggested by @tillrohrmann in the JIRA.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] flink pull request: [FLINK-1979] Lossfunctions
Posted by tillrohrmann <gi...@git.apache.org>.
Github user tillrohrmann commented on the pull request:
https://github.com/apache/flink/pull/656#issuecomment-209818726
Sure go ahead @skavulya.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---