Skip to content

Conversation

@jkbradley
Copy link
Member

Add save/load to LogisticRegression Estimator, and refactor tests a little to make it easier to add similar support to other Estimator, Model pairs.

Moved LogisticRegressionReader/Writer to within LogisticRegressionModel

CC: @mengxr

Copy link
Member Author

Choose a reason for hiding this comment

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

This is awkward, but type issues prevented me from using apply instead of get

@jkbradley
Copy link
Member Author

Oops, I just realized I forgot to save the parent field in both this and Pipeline. Shall we just document that it does not get saved, and handle it for 1.7?

@SparkQA
Copy link

SparkQA commented Nov 17, 2015

Test build #46032 has finished for PR 9749 at commit 528b5b1.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):\n * class BitSet(numBits: Int) extends Serializable\n

Copy link
Contributor

Choose a reason for hiding this comment

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

We won't need this if we implement testDefaultReadWrite properly.

@mengxr
Copy link
Contributor

mengxr commented Nov 17, 2015

LGTM except one minor issue commented inline. It should be okay to postpone saving parent to 1.7.

@jkbradley
Copy link
Member Author

Thanks, updated!

@SparkQA
Copy link

SparkQA commented Nov 17, 2015

Test build #46102 has finished for PR 9749 at commit 54a4885.

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

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need pull in Path from Hadoop. This should work:

val subdir = new File(tempDir, subdirName)
val path = new File(subdir, uid).getPath

@jkbradley
Copy link
Member Author

Rebased to pull PipelineSuite from master.
I removed the failing part of the PipelineSuite. It was not really needed since other tests already check that save/load works and that stages are saved to appropriate directories.

@SparkQA
Copy link

SparkQA commented Nov 17, 2015

Test build #46110 has finished for PR 9749 at commit 1f61d86.

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

asfgit pushed a commit that referenced this pull request Nov 17, 2015
Add save/load to LogisticRegression Estimator, and refactor tests a little to make it easier to add similar support to other Estimator, Model pairs.

Moved LogisticRegressionReader/Writer to within LogisticRegressionModel

CC: mengxr

Author: Joseph K. Bradley <[email protected]>

Closes #9749 from jkbradley/lr-io-2.

(cherry picked from commit 6eb7008)
Signed-off-by: Xiangrui Meng <[email protected]>
@mengxr
Copy link
Contributor

mengxr commented Nov 17, 2015

Merged into master and branch-1.6. Thanks!

@asfgit asfgit closed this in 6eb7008 Nov 17, 2015
@jkbradley jkbradley deleted the lr-io-2 branch November 18, 2015 00:38
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