Skip to content

Conversation

@huaxingao
Copy link
Contributor

What changes were proposed in this pull request?

Deprecate numTrees in GBT in 2.4.5 so it can be removed in 3.0.0

Why are the changes needed?

Currently, GBT has

  /**
   * Number of trees in ensemble
   */
  @Since("2.0.0")
  val getNumTrees: Int = trees.length

and

  /** Number of trees in ensemble */
  val numTrees: Int = trees.length

I think we should remove one of them. I will deprecate it in 2.4.5 and remove it in 3.0.0

Does this PR introduce any user-facing change?

Deprecate numTrees in 2.4.5

How was this patch tested?

Existing tests

@huaxingao
Copy link
Contributor Author

cc @dongjoon-hyun @srowen

/**
* Number of trees in ensemble
*
* @deprecated Use [[getNumTrees]] instead. This method will be removed in 3.0.0
Copy link
Member

Choose a reason for hiding this comment

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

nit. 3.0.0 -> 3.0.0.

/**
* Number of trees in ensemble
*
* @deprecated Use [[getNumTrees]] instead. This method will be removed in 3.0.0
Copy link
Member

Choose a reason for hiding this comment

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

ditto.

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-30630][ML] Deprecate numTrees in GBT in 2.4.5 [SPARK-30630][ML][2.4] Deprecate numTrees in GBT in 2.4.5 Jan 24, 2020
@SparkQA
Copy link

SparkQA commented Jan 24, 2020

Test build #117339 has finished for PR 27352 at commit 7ff296b.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jan 24, 2020

Test build #117341 has finished for PR 27352 at commit 92e12a7.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@huaxingao
Copy link
Contributor Author

Retest this please.

@SparkQA
Copy link

SparkQA commented Jan 24, 2020

Test build #117347 has finished for PR 27352 at commit 92e12a7.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @huaxingao .
Merged to branch-2.4.

dongjoon-hyun pushed a commit that referenced this pull request Jan 24, 2020
### What changes were proposed in this pull request?
Deprecate numTrees in GBT in 2.4.5 so it can be removed in 3.0.0

### Why are the changes needed?
Currently, GBT has
```
  /**
   * Number of trees in ensemble
   */
  Since("2.0.0")
  val getNumTrees: Int = trees.length
```
and
```
  /** Number of trees in ensemble */
  val numTrees: Int = trees.length
```
I think we should remove one of them. I will deprecate it in 2.4.5 and remove it in 3.0.0

### Does this PR introduce any user-facing change?
Deprecate numTrees in 2.4.5

### How was this patch tested?
Existing tests

Closes #27352 from huaxingao/spark-tree.

Authored-by: Huaxin Gao <huaxing@us.ibm.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
@huaxingao
Copy link
Contributor Author

Thank you very much!! @dongjoon-hyun

dongjoon-hyun pushed a commit that referenced this pull request Jan 24, 2020
### What changes were proposed in this pull request?
Remove ```numTrees``` in GBT in 3.0.0.

### Why are the changes needed?
Currently, GBT has
```
  /**
   * Number of trees in ensemble
   */
  Since("2.0.0")
  val getNumTrees: Int = trees.length
```
and
```
  /** Number of trees in ensemble */
  val numTrees: Int = trees.length
```
I think we should remove one of them. We deprecated it in 2.4.5 via #27352.

### Does this PR introduce any user-facing change?
Yes, remove ```numTrees``` in GBT in 3.0.0

### How was this patch tested?
existing tests

Closes #27330 from huaxingao/spark-numTrees.

Authored-by: Huaxin Gao <huaxing@us.ibm.com>
Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants