-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-1892][MLLIB] Adding OWL-QN optimizer for L1 regularizations. It can also handle L2 re... #840
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
jira link : |
|
Can one of the admins verify this patch? |
|
To clarify - it requires the latest breeze. The OWL-QN in breeze had bugs, which I fixed. I'm not sure if David's published an official release yet but it's in the latest snapshot. |
|
I'll try to get David to publish the latest breeze and change the project file to reference the latest breeze. |
|
Breeze has been updated to 0.8. This should now work. |
|
@codedeft make sure breeze is updated in pom.xml as well if this PR is merged in....I pulled in your code to test our internal datasets...in mllib/pom.xml it is still at 0.7..please update it to 0.8... org.scalanlp breeze_${scala.binary.version} 0.7 |
|
Done! |
|
@codedeft could you fix the conflicts when merging into master? |
|
@codedeft Could you add |
… regularizations together. It extends LBFGS. It uses the OWL-QN implementation from breeze (which didn't work correctly before, but it was also fixed prior to this). It requires the latest version of breeze to work correctly.
|
Can one of the admins verify this patch? |
|
@codedeft I am trying to test OWLQN and compare with a C++ baseline on our data but it seems it runs only one iteration...I am setting up 10 iterations and convergenceTol as 1e-4...caller code is simple: class LogisticRegressionWithBFGS private ( Are there any issues with the way Breeze OWLQN is called ? |
|
@debasish83 We fixed the previously broken Breeze OWLQN in Breeze 0.8 and we know that the new Breeze OWLQN works as expected. However, this particular PR does not address a (previously) existing problem in MLLib where regularization is applied to not just weights, but also the intercept. So if you are comparing against results that do L1 regularization properly (i.e. leaving out the intercept from regularization), you'll get different results. I think @dbtsai might be working on the intercept issue. I'm not sure if that's been merged yet. |
|
I am not that much bothered with intercept right now...Say if I force intercept to 0, this should work as expected right ? |
|
@debasish83 |
|
@debasish83 and @codedeft The weighted method for OWLQN in breeze is merged scalanlp/breeze@2570911 I will submit a PR to Spark to use newer version of breeze with this feature once @dlwh publishes to this to maven. But there is still some work in mllib side to have it working properly. I'll work on this once I'm back from vacation. |
|
breeze 0.10 is released. On Wed, Oct 1, 2014 at 2:28 AM, DB Tsai [email protected] wrote:
|
|
Mind closing this PR? |
|
This will be replaced by #1518 (comment) |
Co-authored-by: Egor Krivokon <>
Co-authored-by: Egor Krivokon <>
Co-authored-by: Egor Krivokon <>
Adding OWL-QN optimizer for L1 regularizations. It can also handle L2 and L1 regularizations together (balanced with alpha as in elastic nets). It extends LBFGS. It uses the OWL-QN implementation from breeze (which didn't work correctly before, but it was also fixed prior to this and committed to the latest breeze). Therefore, it requires the latest version of breeze to work correctly.