-
Couldn't load subscription status.
- Fork 28.9k
[SPARK-11852] [ML] StandardScaler minor refactor #9839
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,8 +70,8 @@ class StandardScalerSuite extends SparkFunSuite with MLlibTestSparkContext | |
|
|
||
| test("params") { | ||
| ParamsSuite.checkParams(new StandardScaler) | ||
| val oldModel = new feature.StandardScalerModel(Vectors.dense(1.0), Vectors.dense(2.0)) | ||
| ParamsSuite.checkParams(new StandardScalerModel("empty", oldModel)) | ||
| ParamsSuite.checkParams(new StandardScalerModel("empty", | ||
| Vectors.dense(1.0), Vectors.dense(2.0))) | ||
| } | ||
|
|
||
| test("Standardization with default parameter") { | ||
|
|
@@ -126,13 +126,10 @@ class StandardScalerSuite extends SparkFunSuite with MLlibTestSparkContext | |
| } | ||
|
|
||
| test("StandardScalerModel read/write") { | ||
| val oldModel = new feature.StandardScalerModel( | ||
| Vectors.dense(1.0, 2.0), Vectors.dense(3.0, 4.0), false, true) | ||
| val instance = new StandardScalerModel("myStandardScalerModel", oldModel) | ||
| val instance = new StandardScalerModel("myStandardScalerModel", | ||
| Vectors.dense(1.0, 2.0), Vectors.dense(3.0, 4.0)) | ||
| val newInstance = testDefaultReadWrite(instance) | ||
| assert(newInstance.std === instance.std) | ||
| assert(newInstance.mean === instance.mean) | ||
| assert(newInstance.getWithStd === instance.getWithStd) | ||
| assert(newInstance.getWithMean === instance.getWithMean) | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is not an ideal unit test for read/write because the model fitting part shouldn't be part of it, which is already covered by other tests. Constructing estimator and model directly can save some test time. |
||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If
withMeanandwithStdare parameters, we should save them inmetadata/but not both underdata/andmedadata/. Can we change the constructor ofml.StandardScalerModelto take onlystdandmeanbut constructscaleronly insidetransform? Soscaleris no longer a member variable. We can fix performance issues in 1.7.