Skip to content

Conversation

@dbtsai
Copy link
Member

@dbtsai dbtsai commented Aug 29, 2014

The current loss of the regularization is computed from the newWeights which is not correct. The loss, R(w) = 1/2 ||w||^2 should be computed with the oldWeights.

@SparkQA
Copy link

SparkQA commented Aug 29, 2014

QA tests have started for PR 2207 at commit 1447c23.

  • This patch merges cleanly.

@dbtsai
Copy link
Member Author

dbtsai commented Aug 29, 2014

LBFGS needs correct loss to find next weights while SGD doesn't.

@dbtsai dbtsai closed this Aug 29, 2014
@dbtsai dbtsai reopened this Aug 29, 2014
@SparkQA
Copy link

SparkQA commented Aug 29, 2014

QA tests have finished for PR 2207 at commit 1447c23.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@srowen
Copy link
Member

srowen commented Aug 29, 2014

@dbtsai The scaladoc for Updater says the return value is:

A tuple of 2 elements. The first element is a column matrix containing updated weights,
and the second element is the regularization value computed using updated weights.

Looking at GradientDescent that seems like how it is intended to work, since the loss is computed from the weights and regularization term from the previous iteration. Wouldn't this put it out of sync by 1 iteration? You would have theta_i weights but a regularization term computed over theta_{i-1}.

I might be overlooking something and you know the L-BFGS implementation better but are you sure this should change for both?

@dbtsai
Copy link
Member Author

dbtsai commented Aug 31, 2014

@srowen @mengxr

I was working on OWLQN for L1 in my company, and I didn't follow the LBFGS code so I was confused. The current code in MLlib actually gives the correct result.

The Updater api is a little confusing, and after I read my note when I implemented LBFGS, I actually use the existing Updater api to get the current loss of regularization correctly with a trick by setting the gradient as zero vector, stepSize as zero, and iteration as one.

For SGD, we computed the loss of regularization after weights are updated, and we keep this value, and add it into total loss in next iteration. I now remembered that I fixed a bug because of the updater design couple months ago - the first iteration of the loss of regularization was not properly computed.

Hope the whole design issue can be addressed by #1518 [SPARK-2505][MLlib] Weighted Regularizer for Generalized Linear Model once this is finished.

@dbtsai dbtsai closed this Aug 31, 2014
@dbtsai
Copy link
Member Author

dbtsai commented Aug 31, 2014

PS, it seems that I can not close https://issues.apache.org/jira/browse/SPARK-3317 myself. Can any of you close for me? Thanks.

@srowen
Copy link
Member

srowen commented Aug 31, 2014

@dbtsai are you logged in? That's explained 100% of the many times Apache JIRA stumped me with "why can't I modify my own issue"?

@dbtsai
Copy link
Member Author

dbtsai commented Aug 31, 2014

You are right. Using my desktop without login session. Thanks

@dbtsai dbtsai deleted the dbtsai-updater branch October 28, 2014 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants