[SPARK-13761] [ML] Deprecate validateParams #11620
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Test build #52781 has finished for PR 11620 at commit
|
|
Test build #52822 has finished for PR 11620 at commit
|
|
Test build #52833 has finished for PR 11620 at commit
|
| */ | ||
| @deprecated("All the checks should be merged into transformSchema", "2.0.0") |
There was a problem hiding this comment.
Also say this method will be removed in 2.1.0
|
Only a few small comments |
| @@ -61,7 +63,8 @@ private[regression] trait GeneralizedLinearRegressionBase extends PredictorParam | |||
| * Param for the name of link function which provides the relationship | |||
| * between the linear predictor and the mean of the distribution function. | |||
| * Supported options: "identity", "log", "inverse", "logit", "probit", "cloglog" and "sqrt". | |||
| * @group param | |||
| * | |||
|
Thanks for the updates. Still some indentation issues, but that's it. |
|
Test build #53347 has finished for PR 11620 at commit
|
|
@jkbradley Sorry for those unintentional changes and thanks for the patience. |
|
Test build #53375 has finished for PR 11620 at commit
|
|
No problem. Thanks for the PR! |
| */ | ||
| @deprecated("Will be removed in 2.1.0. Checks should be merged into transformSchema.", "2.0.0") |
There was a problem hiding this comment.
It looks like this now causes a number of deprecation warnings in the Spark code, which we're trying to get rid of. Can most of the remaining usages be transformed to not use this method?
There was a problem hiding this comment.
Apologies! I should have checked the Jenkins logs. I'll send a clean-up PR.
There was a problem hiding this comment.
Hi @jkbradley have you already started on this? Sorry for the troubling. I didn't remove the ones in CrossValidator and TrainValidationSplit because I think it can be handy if we can run some validation before submitting the paramMap. Let me know if I can help in any way.
There was a problem hiding this comment.
No problem; I just sent a PR for it.
asfgit
pushed a commit
that referenced
this pull request
Mar 17, 2016
## What changes were proposed in this pull request? Cleanups from [#11620]: remove remaining uses of validateParams, and put functionality into transformSchema ## How was this patch tested? Existing unit tests, modified to check using transformSchema instead of validateParams Author: Joseph K. Bradley <[email protected]> Closes #11790 from jkbradley/SPARK-13761-cleanup.
roygao94
pushed a commit
to roygao94/spark
that referenced
this pull request
Mar 22, 2016
## What changes were proposed in this pull request? Deprecate validateParams() method here: https://github.com/apache/spark/blob/035d3acdf3c1be5b309a861d5c5beb803b946b5e/mllib/src/main/scala/org/apache/spark/ml/param/params.scala#L553 Move all functionality in overridden methods to transformSchema(). Check docs to make sure they indicate complex Param interaction checks should be done in transformSchema. ## How was this patch tested? unit tests Author: Yuhao Yang <[email protected]> Closes apache#11620 from hhbyyh/depreValid.
roygao94
pushed a commit
to roygao94/spark
that referenced
this pull request
Mar 22, 2016
## What changes were proposed in this pull request? Cleanups from [apache#11620]: remove remaining uses of validateParams, and put functionality into transformSchema ## How was this patch tested? Existing unit tests, modified to check using transformSchema instead of validateParams Author: Joseph K. Bradley <[email protected]> Closes apache#11790 from jkbradley/SPARK-13761-cleanup.
ghost
pushed a commit
to dbtsai/spark
that referenced
this pull request
Sep 26, 2016
…hon API. ## What changes were proposed in this pull request? apache#14597 modified ```ChiSqSelector``` to support ```fpr``` type selector, however, it left some issue need to be addressed: * We should allow users to set selector type explicitly rather than switching them by using different setting function, since the setting order will involves some unexpected issue. For example, if users both set ```numTopFeatures``` and ```percentile```, it will train ```kbest``` or ```percentile``` model based on the order of setting (the latter setting one will be trained). This make users confused, and we should allow users to set selector type explicitly. We handle similar issues at other place of ML code base such as ```GeneralizedLinearRegression``` and ```LogisticRegression```. * Meanwhile, if there are more than one parameter except ```alpha``` can be set for ```fpr``` model, we can not handle it elegantly in the existing framework. And similar issues for ```kbest``` and ```percentile``` model. Setting selector type explicitly can solve this issue also. * If setting selector type explicitly by users is allowed, we should handle param interaction such as if users set ```selectorType = percentile``` and ```alpha = 0.1```, we should notify users the parameter ```alpha``` will take no effect. We should handle complex parameter interaction checks at ```transformSchema```. (FYI apache#11620) * We should use lower case of the selector type names to follow MLlib convention. * Add ML Python API. ## How was this patch tested? Unit test. Author: Yanbo Liang <[email protected]> Closes apache#15214 from yanboliang/spark-17017.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
Deprecate validateParams() method here:
spark/mllib/src/main/scala/org/apache/spark/ml/param/params.scala
Line 553 in 035d3ac
Move all functionality in overridden methods to transformSchema().
Check docs to make sure they indicate complex Param interaction checks should be done in transformSchema.
How was this patch tested?
unit tests