Skip to content

[SPARK-6164] [ML] CrossValidatorModel should keep stats from fitting #5915

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 8 commits into from
Closed

[SPARK-6164] [ML] CrossValidatorModel should keep stats from fitting #5915

wants to merge 8 commits into from

Conversation

@leahmcguire
Copy link
Contributor

Added stats from cross validation as a val in the cross validation model to save them for user access.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@jkbradley
Copy link
Member

@leahmcguire Could you please try closing & re-opening this PR to make Github recompute the difference between this & the master branch? If that does not fix the computed diff, then perhaps rebase? Thanks!

@leahmcguire leahmcguire closed this May 7, 2015
@leahmcguire leahmcguire reopened this May 7, 2015
@leahmcguire
Copy link
Contributor Author

Fixed

jkbradley
jkbradley reviewed May 8, 2015
View reviewed changes
mllib/src/main/scala/org/apache/spark/ml/tuning/CrossValidator.scala Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please add doc, such as Metric for each parameter setting tested during cross-validation, corresponding to the settings in parent.estimatorParamMaps. Each value is the average over numFolds trials.

Also, do you think "crossValidationMetrics" is too long a name, especially since it's clear it has to do with cross-validation? How about "avgMetrics?"

@jkbradley
Copy link
Member

That's all. We should be able to test & merge for 1.4. Thanks!

@jkbradley
Copy link
Member

ok to test

@jkbradley
Copy link
Member

@leahmcguire I'm sorry for letting this slip! I don't know why I didn't get a Github notification about your updates.

Ping @mengxr Think it's OK if we merge this for 1.4? It would be really nice to have available.

@SparkQA
Copy link

SparkQA commented Jun 2, 2015

Test build #871 has started for PR 5915 at commit 15d1015.

@jkbradley
Copy link
Member

Uh oh, those tests won't work because of merge conflicts.

@leahmcguire
Copy link
Contributor Author

l will try to resolve and update the pull request.

On Tue, Jun 2, 2015 at 10:49 AM, jkbradley [email protected] wrote:

Uh oh, those tests won't work because of merge conflicts.

—
Reply to this email directly or view it on GitHub
#5915 (comment).

@SparkQA
Copy link

SparkQA commented Jun 2, 2015

Test build #871 timed out for PR 5915 at commit 15d1015 after a configured wait of 150m.

@leahmcguire
Copy link
Contributor Author

Ok, think I fixed the merge and cleaned up the pull request so it is just my files.

@SparkQA
Copy link

SparkQA commented Jun 2, 2015

Test build #873 has started for PR 5915 at commit 67537b1.

@SparkQA
Copy link

SparkQA commented Jun 2, 2015

Test build #873 has finished for PR 5915 at commit 67537b1.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 2, 2015

Test build #875 has started for PR 5915 at commit 49b507b.

@SparkQA
Copy link

SparkQA commented Jun 2, 2015

Test build #875 has finished for PR 5915 at commit 49b507b.

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

@SparkQA
Copy link

SparkQA commented Jun 2, 2015

Test build #876 has started for PR 5915 at commit 49b507b.

@SparkQA
Copy link

SparkQA commented Jun 3, 2015

Test build #876 has finished for PR 5915 at commit 49b507b.

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

@jkbradley
Copy link
Member

These failures are really strange. I don't see them happening to other PRs or the master build, but this PR can't be causing them. I'll test again...

@jkbradley
Copy link
Member

test this please

@SparkQA
Copy link

SparkQA commented Jun 3, 2015

Test build #877 has started for PR 5915 at commit 49b507b.

@SparkQA
Copy link

SparkQA commented Jun 3, 2015

Test build #877 timed out for PR 5915 at commit 49b507b after a configured wait of 150m.

@jkbradley
Copy link
Member

test this please

@leahmcguire
Copy link
Contributor Author

Anything I should try to do to fix this?

@jkbradley
Copy link
Member

@leahmcguire I'm a bit confused. All of these test failure seem unrelated, and that last one seemed to work but failed when posting to Github. I'm going to try running once more.... My feeling is that Jenkins was being grumpy.

@SparkQA
Copy link

SparkQA commented Jun 3, 2015

Test build #878 has started for PR 5915 at commit 49b507b.

@SparkQA
Copy link

SparkQA commented Jun 3, 2015

Test build #878 has finished for PR 5915 at commit 49b507b.

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

@leahmcguire
Copy link
Contributor Author

Yay!

@jkbradley
Copy link
Member

Finally! I'll merge this into master
Thank you!

@asfgit asfgit closed this in d8662cd Jun 3, 2015
jeanlyn pushed a commit to jeanlyn/spark that referenced this pull request Jun 12, 2015
@leahmcguire @jeanlyn
[SPARK-6164] [ML] CrossValidatorModel should keep stats from fitting
6294e6e 
Added stats from cross validation as a val in the cross validation model to save them for user access.

Author: leahmcguire <[email protected]>

Closes apache#5915 from leahmcguire/saveCVmetrics and squashes the following commits:

49b507b [leahmcguire] fixed tyle error
67537b1 [leahmcguire] rebased
85907f0 [leahmcguire] fixed name
59987cc [leahmcguire] changed param name and test according to comments
36e71e3 [leahmcguire] rebasing
4b8223e [leahmcguire] fixed name
4ddffc6 [leahmcguire] changed param name and test according to comments
3a995da [leahmcguire] Added stats from cross validation as a val in the cross validation model to save them for user access
nemccarthy pushed a commit to nemccarthy/spark that referenced this pull request Jun 19, 2015
@leahmcguire @nemccarthy
[SPARK-6164] [ML] CrossValidatorModel should keep stats from fitting
b017d55 
Added stats from cross validation as a val in the cross validation model to save them for user access.

Author: leahmcguire <[email protected]>

Closes apache#5915 from leahmcguire/saveCVmetrics and squashes the following commits:

49b507b [leahmcguire] fixed tyle error
67537b1 [leahmcguire] rebased
85907f0 [leahmcguire] fixed name
59987cc [leahmcguire] changed param name and test according to comments
36e71e3 [leahmcguire] rebasing
4b8223e [leahmcguire] fixed name
4ddffc6 [leahmcguire] changed param name and test according to comments
3a995da [leahmcguire] Added stats from cross validation as a val in the cross validation model to save them for user access
viirya referenced this pull request Jun 22, 2015
@viirya @jkbradley
[SPARK-8468] [ML] Take the negative of some metrics in RegressionEval…
fe59a4a 
…uator to get correct cross validation

JIRA: https://issues.apache.org/jira/browse/SPARK-8468

Author: Liang-Chi Hsieh <[email protected]>

Closes #6905 from viirya/cv_min and squashes the following commits:

930d3db [Liang-Chi Hsieh] Fix python unit test and add document.
d632135 [Liang-Chi Hsieh] Merge remote-tracking branch 'upstream/master' into cv_min
16e3b2c [Liang-Chi Hsieh] Take the negative instead of reciprocal.
c3dd8d9 [Liang-Chi Hsieh] For comments.
b5f52c1 [Liang-Chi Hsieh] Add param to CrossValidator for choosing whether to maximize evaulation value.

(cherry picked from commit 0b89951)
Signed-off-by: Joseph K. Bradley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@leahmcguire @AmplabJenkins @jkbradley @SparkQA