Repository: scikit-learn/scikit-learn
Pull request: BUG max_depth=1 should be decision stump in HistGradientBoosti…

#### Reference Issues/PRs
Fixes #16124

#### What does this implement/fix? Explain your changes.
Modified the validation in TreeGrower to allow max_depth value as 1.

Problem statement:
Off-by-one in HistGradientBoosting max_depth
Right now HistGradientBoostingClassifier doesn't allow max_depth=1.
Either this is an issue just in the validation, or there's an off-by-one in the code. Max-depth 1 should be a stump, i.e. a single split, and should totally be allowed.

cc @NicolasHug

Hints from issue discussion:
The convention for the depth is that depth=1 corresponds to a single (non-splitted) node which would predict the average target so there's no point in allowing that
uhm this is not the convention in the other tree algorithm. We might want to homogenize it.
FWIW, the parameter is properly documented:

> The maximum depth of each tree. The depth of a tree is the number of
  nodes to go from the root to the deepest leaf. Must be strictly greater
  than 1. Depth isn't constrained by default.

IMHO this convention makes more sense, but I agree we should stay consistent with the rest of the code base
Yeah well the definition is not that definition:
https://en.wikipedia.org/wiki/Tree-depth
https://en.wikipedia.org/wiki/Tree_(data_structure)#Terminology

Commit message: Fixed the test for max_depth in test_gradient_boosting.py

Patch to review:
diff --git a/sklearn/ensemble/_hist_gradient_boosting/gradient_boosting.py b/sklearn/ensemble/_hist_gradient_boosting/gradient_boosting.py
index cb708ecc576e7..922e53eef86a7 100644
--- a/sklearn/ensemble/_hist_gradient_boosting/gradient_boosting.py
+++ b/sklearn/ensemble/_hist_gradient_boosting/gradient_boosting.py
@@ -689,8 +689,7 @@ class HistGradientBoostingRegressor(RegressorMixin, BaseHistGradientBoosting):
         than 1. If None, there is no maximum limit.
     max_depth : int or None, optional (default=None)
         The maximum depth of each tree. The depth of a tree is the number of
-        nodes to go from the root to the deepest leaf. Must be strictly greater
-        than 1. Depth isn't constrained by default.
+        nodes to go from the root to the deepest leaf. Depth isn't constrained by default.
     min_samples_leaf : int, optional (default=20)
         The minimum number of samples per leaf. For small datasets with less
         than a few hundred samples, it is recommended to lower this value
diff --git a/sklearn/ensemble/_hist_gradient_boosting/grower.py b/sklearn/ensemble/_hist_gradient_boosting/grower.py
index c7d303b8f6201..5e1380ecb1e54 100644
--- a/sklearn/ensemble/_hist_gradient_boosting/grower.py
+++ b/sklearn/ensemble/_hist_gradient_boosting/grower.py
@@ -230,9 +230,9 @@ def _validate_parameters(self, X_binned, max_leaf_nodes, max_depth,
         if max_leaf_nodes is not None and max_leaf_nodes <= 1:
             raise ValueError('max_leaf_nodes={} should not be'
                              ' smaller than 2'.format(max_leaf_nodes))
-        if max_depth is not None and max_depth <= 1:
+        if max_depth is not None and max_depth < 1:
             raise ValueError('max_depth={} should not be'
-                             ' smaller than 2'.format(max_depth))
+                             ' smaller than 1'.format(max_depth))
         if min_samples_leaf < 1:
             raise ValueError('min_samples_leaf={} should '
                              'not be smaller than 1'.format(min_samples_leaf))
diff --git a/sklearn/ensemble/_hist_gradient_boosting/tests/test_gradient_boosting.py b/sklearn/ensemble/_hist_gradient_boosting/tests/test_gradient_boosting.py
index 87950eab38a97..b607cdd23b6c9 100644
--- a/sklearn/ensemble/_hist_gradient_boosting/tests/test_gradient_boosting.py
+++ b/sklearn/ensemble/_hist_gradient_boosting/tests/test_gradient_boosting.py
@@ -31,8 +31,7 @@
      ({'max_iter': 0}, 'max_iter=0 must not be smaller than 1'),
      ({'max_leaf_nodes': 0}, 'max_leaf_nodes=0 should not be smaller than 2'),
      ({'max_leaf_nodes': 1}, 'max_leaf_nodes=1 should not be smaller than 2'),
-     ({'max_depth': 0}, 'max_depth=0 should not be smaller than 2'),
-     ({'max_depth': 1}, 'max_depth=1 should not be smaller than 2'),
+     ({'max_depth': 0}, 'max_depth=0 should not be smaller than 1'),
      ({'min_samples_leaf': 0}, 'min_samples_leaf=0 should not be smaller'),
      ({'l2_regularization': -1}, 'l2_regularization=-1 must be positive'),
      ({'max_bins': 1}, 'max_bins=1 should be no smaller than 2 and no larger'),
diff --git a/sklearn/ensemble/_hist_gradient_boosting/tests/test_grower.py b/sklearn/ensemble/_hist_gradient_boosting/tests/test_grower.py
index 0cc301b7b1b36..a5bdeb5cc71f3 100644
--- a/sklearn/ensemble/_hist_gradient_boosting/tests/test_grower.py
+++ b/sklearn/ensemble/_hist_gradient_boosting/tests/test_grower.py
@@ -257,7 +257,7 @@ def test_min_samples_leaf_root(n_samples, min_samples_leaf):
         assert len(grower.finalized_leaves) == 1
 
 
-@pytest.mark.parametrize('max_depth', [2, 3])
+@pytest.mark.parametrize('max_depth', [1, 2, 3])
 def test_max_depth(max_depth):
     # Make sure max_depth parameter works as expected
     rng = np.random.RandomState(seed=0)


Code review comments:
diff_hunk: @@ -689,8 +689,7 @@ class HistGradientBoostingRegressor(RegressorMixin, BaseHistGradientBoosting):
         than 1. If None, there is no maximum limit.
     max_depth : int or None, optional (default=None)
         The maximum depth of each tree. The depth of a tree is the number of
-        nodes to go from the root to the deepest leaf. Must be strictly greater
-        than 1. Depth isn't constrained by default.
+        nodes to go from the root to the deepest leaf. Depth isn't constrained by default.
path: sklearn/ensemble/_hist_gradient_boosting/gradient_boosting.py
text: @user1:
```suggestion
        edges to go from the root to the deepest leaf. Depth isn't constrained by default.
```

@user1:
Please also update the classifier's docstring 

@author:
@user1 Thanks for the review. I have updated the docstrings and also added a test accordingly.