You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by HyukjinKwon <gi...@git.apache.org> on 2016/06/11 16:55:11 UTC

[GitHub] spark pull request #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregato...

GitHub user HyukjinKwon opened a pull request:

    https://github.com/apache/spark/pull/13619

    [SPARK-15892][ML] Incorrectly merged AFTAggregator with zero total count

    ## What changes were proposed in this pull request?
    
    Currently, `AFTAggregator` is not being merged correctly. For example, if there is any single empty partition in the data, this creates an `AFTAggregator` with zero total count which causes the exception below:
    
    ```
    IllegalArgumentException: u'requirement failed: The number of instances should be greater than 0.0, but got 0.'
    ```
    
    Please see [AFTSurvivalRegression.scala#L573-L575](https://github.com/apache/spark/blob/6ecedf39b44c9acd58cdddf1a31cf11e8e24428c/mllib/src/main/scala/org/apache/spark/ml/regression/AFTSurvivalRegression.scala#L573-L575) as well.
    
    
    Just to be clear, the python example `aft_survival_regression.py` seems using 5 rows. So, if there exist partitions more than 5, it throws the exception above since it contains empty partitions which results in an incorrectly merged `AFTAggregator`.
    
    Executing `bin/spark-submit examples/src/main/python/ml/aft_survival_regression.py` on a machine with CPUs more than 5 is being failed because it creates tasks with some empty partitions.
    
    ## How was this patch tested?
    
    Manually tested by `bin/spark-submit examples/src/main/python/ml/aft_survival_regression.py`.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/HyukjinKwon/spark SPARK-15892

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/13619.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #13619
    
----
commit fb16b71a96ef55541207b77c9bb9bc49d0a85243
Author: hyukjinkwon <gu...@gmail.com>
Date:   2016-06-11T16:40:04Z

    Fix incorrect comparison

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    Reverted commit to branch-1.6 in commit 2f3e327c4cbf163d8536c4451b4829ec7d1886a9


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    Yes, it seems it is still failing. I can change the PR to revert this if he is busy for now. Otherwise, I will close mine.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    @jkbradley Sure!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregato...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/13619


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    @HyukjinKwon OK, but this pr should be reverted first.
    @jkbradley  could you revert this pr for branch-1.6 ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    @zzcclp I made a PR against branch-1.6 here, https://github.com/apache/spark/pull/13630. Thank you for pointing this out quickly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    @zzcclp I just did here https://github.com/apache/spark/pull/13630


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregato...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13619#discussion_r66732911
  
    --- Diff: mllib/src/test/scala/org/apache/spark/ml/regression/AFTSurvivalRegressionSuite.scala ---
    @@ -390,6 +390,18 @@ class AFTSurvivalRegressionSuite
         testEstimatorAndModelReadWrite(aft, datasetMultivariate,
           AFTSurvivalRegressionSuite.allParamSettings, checkModelData)
       }
    +
    +  test("SPARK-15892: Incorrectly merged AFTAggregator with zero total count") {
    +    // This `dataset` will contain an empty partition because it has two rows but
    +    // the parallelism is bigger than that. Because the issue was about `AFTAggregator`s
    +    // being merged incorrectly when it has an empty partition, running the codes below
    +    // should not throw an exception.
    +    val dataset = spark.createDataFrame(
    --- End diff --
    
    with branch-2.0, it is OK , i think that this pr should not be merged into branch-1.6 directly.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    **[Test build #60345 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60345/consoleFull)** for PR 13619 at commit [`fb16b71`](https://github.com/apache/spark/commit/fb16b71a96ef55541207b77c9bb9bc49d0a85243).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    @HyukjinKwon , just see it, thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60345/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    My apologies for the slow response.  I'll revert it now.  Thank you for pinging!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    **[Test build #60353 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60353/consoleFull)** for PR 13619 at commit [`4447d0a`](https://github.com/apache/spark/commit/4447d0a969229dfe5d6cf1bdfc3c0ac62c1fd53e).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    @HyukjinKwon ,  could you revert this pr for branch-1.6 ? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60357/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60359/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    **[Test build #60357 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60357/consoleFull)** for PR 13619 at commit [`c86ede8`](https://github.com/apache/spark/commit/c86ede8fc913be2713d5f6794741f9fbc51f5169).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    @HyukjinKwon Thanks!  The fix looks correct.  Could you please add a tiny unit test which fails before your fix & works afterwards?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    **[Test build #60345 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60345/consoleFull)** for PR 13619 at commit [`fb16b71`](https://github.com/apache/spark/commit/fb16b71a96ef55541207b77c9bb9bc49d0a85243).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    **[Test build #60359 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60359/consoleFull)** for PR 13619 at commit [`2c8adbf`](https://github.com/apache/spark/commit/2c8adbfe53493b4bcef1a633ab429f6612c67fe5).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by jkbradley <gi...@git.apache.org>.
Github user jkbradley commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    LGTM
    Merging with master and branch-2.0
    Thank you!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregato...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13619#discussion_r66732825
  
    --- Diff: mllib/src/test/scala/org/apache/spark/ml/regression/AFTSurvivalRegressionSuite.scala ---
    @@ -390,6 +390,18 @@ class AFTSurvivalRegressionSuite
         testEstimatorAndModelReadWrite(aft, datasetMultivariate,
           AFTSurvivalRegressionSuite.allParamSettings, checkModelData)
       }
    +
    +  test("SPARK-15892: Incorrectly merged AFTAggregator with zero total count") {
    +    // This `dataset` will contain an empty partition because it has two rows but
    +    // the parallelism is bigger than that. Because the issue was about `AFTAggregator`s
    +    // being merged incorrectly when it has an empty partition, running the codes below
    +    // should not throw an exception.
    +    val dataset = spark.createDataFrame(
    --- End diff --
    
    I compile it in branch-1.6.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregato...

Posted by zzcclp <gi...@git.apache.org>.
Github user zzcclp commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13619#discussion_r66732808
  
    --- Diff: mllib/src/test/scala/org/apache/spark/ml/regression/AFTSurvivalRegressionSuite.scala ---
    @@ -390,6 +390,18 @@ class AFTSurvivalRegressionSuite
         testEstimatorAndModelReadWrite(aft, datasetMultivariate,
           AFTSurvivalRegressionSuite.allParamSettings, checkModelData)
       }
    +
    +  test("SPARK-15892: Incorrectly merged AFTAggregator with zero total count") {
    +    // This `dataset` will contain an empty partition because it has two rows but
    +    // the parallelism is bigger than that. Because the issue was about `AFTAggregator`s
    +    // being merged incorrectly when it has an empty partition, running the codes below
    +    // should not throw an exception.
    +    val dataset = spark.createDataFrame(
    --- End diff --
    
    @HyukjinKwon value `spark` is not found here, it should be `sqlContext`, right? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    cc @jkbradley (the cause was quite deeper than I thought).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregato...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13619#discussion_r66732958
  
    --- Diff: mllib/src/test/scala/org/apache/spark/ml/regression/AFTSurvivalRegressionSuite.scala ---
    @@ -390,6 +390,18 @@ class AFTSurvivalRegressionSuite
         testEstimatorAndModelReadWrite(aft, datasetMultivariate,
           AFTSurvivalRegressionSuite.allParamSettings, checkModelData)
       }
    +
    +  test("SPARK-15892: Incorrectly merged AFTAggregator with zero total count") {
    +    // This `dataset` will contain an empty partition because it has two rows but
    +    // the parallelism is bigger than that. Because the issue was about `AFTAggregator`s
    +    // being merged incorrectly when it has an empty partition, running the codes below
    +    // should not throw an exception.
    +    val dataset = spark.createDataFrame(
    --- End diff --
    
    Oh, it seems this is merged into branch-1.6 too. Yes, it should be `sqlContext` for branch-1.6.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    FYI, it passes in branch-1.6 because it does not check whether the count is 0 or not. This checking is introduced in https://github.com/apache/spark/commit/101663f1ae222a919fc40510aa4f2bad22d1be6f.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    **[Test build #60359 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60359/consoleFull)** for PR 13619 at commit [`2c8adbf`](https://github.com/apache/spark/commit/2c8adbfe53493b4bcef1a633ab429f6612c67fe5).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    **[Test build #60357 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60357/consoleFull)** for PR 13619 at commit [`c86ede8`](https://github.com/apache/spark/commit/c86ede8fc913be2713d5f6794741f9fbc51f5169).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    **[Test build #60353 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/60353/consoleFull)** for PR 13619 at commit [`4447d0a`](https://github.com/apache/spark/commit/4447d0a969229dfe5d6cf1bdfc3c0ac62c1fd53e).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/60353/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #13619: [SPARK-15892][ML] Incorrectly merged AFTAggregator with ...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/13619
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org