You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/04/20 09:59:27 UTC
[GitHub] [spark] WeichenXu123 opened a new pull request #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
WeichenXu123 opened a new pull request #28273:
URL: https://github.com/apache/spark/pull/28273
### What changes were proposed in this pull request?
Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
### Does this PR introduce any user-facing change?
<!--
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If no, write 'No'.
-->
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28273:
URL: https://github.com/apache/spark/pull/28273#issuecomment-616443757
**[Test build #121520 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121520/testReport)** for PR 28273 at commit [`2af493a`](https://github.com/apache/spark/commit/2af493a016093d8499bf02bc37bc3e842ed74c62).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WeichenXu123 commented on a change in pull request #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
Posted by GitBox <gi...@apache.org>.
WeichenXu123 commented on a change in pull request #28273:
URL: https://github.com/apache/spark/pull/28273#discussion_r411258017
##########
File path: python/pyspark/ml/pipeline.py
##########
@@ -174,6 +174,50 @@ def _to_java(self):
return _java_obj
+ def _make_java_param_pair(self, param, value):
+ """
+ Makes a Java param pair.
+ """
+ sc = SparkContext._active_spark_context
+ param = self._resolveParam(param)
+ java_param = sc._jvm.org.apache.spark.ml.param.Param(param.parent, param.name, param.doc)
+ if isinstance(value, JavaParams):
+ # used in the case of an estimator having another estimator as a parameter
+ # the reason why this is not in _py2java in common.py is that importing
+ # Estimator and Model in common.py results in a circular import with inherit_doc
+ java_value = value._to_java()
+ else:
+ java_value = _py2java(sc, value)
+ return java_param.w(java_value)
+
+ def _transfer_param_map_to_java(self, pyParamMap):
+ """
+ Transforms a Python ParamMap into a Java ParamMap.
+ """
+ paramMap = JavaWrapper._new_java_obj("org.apache.spark.ml.param.ParamMap")
+ for param in self.params:
+ if param in pyParamMap:
+ pair = self._make_java_param_pair(param, pyParamMap[param])
+ paramMap.put([pair])
+ return paramMap
+
+ def _transfer_param_map_from_java(self, javaParamMap):
+ """
+ Transforms a Java ParamMap into a Python ParamMap.
+ """
+ sc = SparkContext._active_spark_context
+ paramMap = dict()
+ for pair in javaParamMap.toList():
+ param = pair.param()
+ if self.hasParam(str(param.name())):
+ java_obj = pair.value()
+ if sc._jvm.Class.forName("org.apache.spark.ml.PipelineStage").isInstance(java_obj):
+ py_obj = JavaParams._from_java(java_obj)
+ else:
+ py_obj = _java2py(sc, java_obj)
+ paramMap[self.getParam(param.name())] = py_obj
+ return paramMap
Review comment:
@mengxr Discussion: Should we put these method into a base class ? So that we can eliminate some duplicated code in `JavaParams`, `OneVsRest`, `Pipeline` class. But where to put them need discussion.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WeichenXu123 commented on a change in pull request #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
Posted by GitBox <gi...@apache.org>.
WeichenXu123 commented on a change in pull request #28273:
URL: https://github.com/apache/spark/pull/28273#discussion_r411258017
##########
File path: python/pyspark/ml/pipeline.py
##########
@@ -174,6 +174,50 @@ def _to_java(self):
return _java_obj
+ def _make_java_param_pair(self, param, value):
+ """
+ Makes a Java param pair.
+ """
+ sc = SparkContext._active_spark_context
+ param = self._resolveParam(param)
+ java_param = sc._jvm.org.apache.spark.ml.param.Param(param.parent, param.name, param.doc)
+ if isinstance(value, JavaParams):
+ # used in the case of an estimator having another estimator as a parameter
+ # the reason why this is not in _py2java in common.py is that importing
+ # Estimator and Model in common.py results in a circular import with inherit_doc
+ java_value = value._to_java()
+ else:
+ java_value = _py2java(sc, value)
+ return java_param.w(java_value)
+
+ def _transfer_param_map_to_java(self, pyParamMap):
+ """
+ Transforms a Python ParamMap into a Java ParamMap.
+ """
+ paramMap = JavaWrapper._new_java_obj("org.apache.spark.ml.param.ParamMap")
+ for param in self.params:
+ if param in pyParamMap:
+ pair = self._make_java_param_pair(param, pyParamMap[param])
+ paramMap.put([pair])
+ return paramMap
+
+ def _transfer_param_map_from_java(self, javaParamMap):
+ """
+ Transforms a Java ParamMap into a Python ParamMap.
+ """
+ sc = SparkContext._active_spark_context
+ paramMap = dict()
+ for pair in javaParamMap.toList():
+ param = pair.param()
+ if self.hasParam(str(param.name())):
+ java_obj = pair.value()
+ if sc._jvm.Class.forName("org.apache.spark.ml.PipelineStage").isInstance(java_obj):
+ py_obj = JavaParams._from_java(java_obj)
+ else:
+ py_obj = _java2py(sc, java_obj)
+ paramMap[self.getParam(param.name())] = py_obj
+ return paramMap
Review comment:
@mengxr Discussion: Should we put these method into a base class (but should not add them in `JavaParams`) ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WeichenXu123 commented on issue #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
Posted by GitBox <gi...@apache.org>.
WeichenXu123 commented on issue #28273:
URL: https://github.com/apache/spark/pull/28273#issuecomment-616970362
Reopen it in here
https://github.com/apache/spark/pull/28279
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] mengxr commented on a change in pull request #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
Posted by GitBox <gi...@apache.org>.
mengxr commented on a change in pull request #28273:
URL: https://github.com/apache/spark/pull/28273#discussion_r411462216
##########
File path: python/pyspark/ml/pipeline.py
##########
@@ -174,6 +174,50 @@ def _to_java(self):
return _java_obj
+ def _make_java_param_pair(self, param, value):
+ """
+ Makes a Java param pair.
+ """
+ sc = SparkContext._active_spark_context
+ param = self._resolveParam(param)
+ java_param = sc._jvm.org.apache.spark.ml.param.Param(param.parent, param.name, param.doc)
+ if isinstance(value, JavaParams):
+ # used in the case of an estimator having another estimator as a parameter
+ # the reason why this is not in _py2java in common.py is that importing
+ # Estimator and Model in common.py results in a circular import with inherit_doc
+ java_value = value._to_java()
+ else:
+ java_value = _py2java(sc, value)
+ return java_param.w(java_value)
+
+ def _transfer_param_map_to_java(self, pyParamMap):
+ """
+ Transforms a Python ParamMap into a Java ParamMap.
+ """
+ paramMap = JavaWrapper._new_java_obj("org.apache.spark.ml.param.ParamMap")
+ for param in self.params:
+ if param in pyParamMap:
+ pair = self._make_java_param_pair(param, pyParamMap[param])
+ paramMap.put([pair])
+ return paramMap
+
+ def _transfer_param_map_from_java(self, javaParamMap):
+ """
+ Transforms a Java ParamMap into a Python ParamMap.
+ """
+ sc = SparkContext._active_spark_context
+ paramMap = dict()
+ for pair in javaParamMap.toList():
+ param = pair.param()
+ if self.hasParam(str(param.name())):
+ java_obj = pair.value()
+ if sc._jvm.Class.forName("org.apache.spark.ml.PipelineStage").isInstance(java_obj):
+ py_obj = JavaParams._from_java(java_obj)
+ else:
+ py_obj = _java2py(sc, java_obj)
+ paramMap[self.getParam(param.name())] = py_obj
+ return paramMap
Review comment:
I don't have a strong preference because they are private methods. How about making a new mix-in called JavaParamsOps that contains those methods and then make JavaParams(JavaWrapper, JavaParamsOps, Params). We don't have to do it in this PR, which will unnecessarily increase the scope.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28273:
URL: https://github.com/apache/spark/pull/28273#issuecomment-616463982
**[Test build #121520 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121520/testReport)** for PR 28273 at commit [`2af493a`](https://github.com/apache/spark/commit/2af493a016093d8499bf02bc37bc3e842ed74c62).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28273:
URL: https://github.com/apache/spark/pull/28273#issuecomment-616444488
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28273:
URL: https://github.com/apache/spark/pull/28273#issuecomment-616443757
**[Test build #121520 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121520/testReport)** for PR 28273 at commit [`2af493a`](https://github.com/apache/spark/commit/2af493a016093d8499bf02bc37bc3e842ed74c62).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] WeichenXu123 commented on a change in pull request #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
Posted by GitBox <gi...@apache.org>.
WeichenXu123 commented on a change in pull request #28273:
URL: https://github.com/apache/spark/pull/28273#discussion_r411258017
##########
File path: python/pyspark/ml/pipeline.py
##########
@@ -174,6 +174,50 @@ def _to_java(self):
return _java_obj
+ def _make_java_param_pair(self, param, value):
+ """
+ Makes a Java param pair.
+ """
+ sc = SparkContext._active_spark_context
+ param = self._resolveParam(param)
+ java_param = sc._jvm.org.apache.spark.ml.param.Param(param.parent, param.name, param.doc)
+ if isinstance(value, JavaParams):
+ # used in the case of an estimator having another estimator as a parameter
+ # the reason why this is not in _py2java in common.py is that importing
+ # Estimator and Model in common.py results in a circular import with inherit_doc
+ java_value = value._to_java()
+ else:
+ java_value = _py2java(sc, value)
+ return java_param.w(java_value)
+
+ def _transfer_param_map_to_java(self, pyParamMap):
+ """
+ Transforms a Python ParamMap into a Java ParamMap.
+ """
+ paramMap = JavaWrapper._new_java_obj("org.apache.spark.ml.param.ParamMap")
+ for param in self.params:
+ if param in pyParamMap:
+ pair = self._make_java_param_pair(param, pyParamMap[param])
+ paramMap.put([pair])
+ return paramMap
+
+ def _transfer_param_map_from_java(self, javaParamMap):
+ """
+ Transforms a Java ParamMap into a Python ParamMap.
+ """
+ sc = SparkContext._active_spark_context
+ paramMap = dict()
+ for pair in javaParamMap.toList():
+ param = pair.param()
+ if self.hasParam(str(param.name())):
+ java_obj = pair.value()
+ if sc._jvm.Class.forName("org.apache.spark.ml.PipelineStage").isInstance(java_obj):
+ py_obj = JavaParams._from_java(java_obj)
+ else:
+ py_obj = _java2py(sc, java_obj)
+ paramMap[self.getParam(param.name())] = py_obj
+ return paramMap
Review comment:
@mengxr Discussion: Should we put these method into a base class (but should not add them in `JavaParams`) ? So that we can eliminate some duplicated code in `JavaParams`, `OneVsRest`, `Pipeline` class. But where to put them need discussion.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28273:
URL: https://github.com/apache/spark/pull/28273#issuecomment-616464468
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28273:
URL: https://github.com/apache/spark/pull/28273#issuecomment-616444488
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28273: [WIP][SPARK-31497][ML][PYSPARK] Fix Pyspark CrossValidator/TrainValidationSplit with pipeline estimator cannot save and load model
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28273:
URL: https://github.com/apache/spark/pull/28273#issuecomment-616464468
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org