FIX max_leaf_node and max_depth interaction in GBDT (scikit-learn#16183)

* fix max_leaf_node max_depth interaction

* Added test

* comment

* what's new

* simpler solution

* moved and simplified test

* typo

Author: NicolasHug

doc/whats_new/v0.23.rst

@@ -112,6 +112,11 @@ Changelog
   :user:`Reshama Shaikh <reshamas>`, and
   :user:`Chiara Marmo <cmarmo>`.
 
+- |API| Fixed a bug in :class:`ensemble.HistGradientBoostingClassifier` and
+  :class:`ensemble.HistGradientBoostingRegrerssor` that would not respect the
+  `max_leaf_nodes` parameter if the criteria was reached at the same time as
+  the `max_depth` criteria. :pr:`16183` by `Nicolas Hug`_.
+
 - |Fix|  Changed the convention for `max_depth` parameter of
   :class:`ensemble.HistGradientBoostingClassifier` and
   :class:`ensemble.HistGradientBoostingRegressor`. The depth now corresponds to

sklearn/ensemble/_hist_gradient_boosting/grower.py

@@ -355,16 +355,16 @@ def split_next(self):
 
         self.n_nodes += 2
 
-        if self.max_depth is not None and depth == self.max_depth:
+        if (self.max_leaf_nodes is not None
+                and n_leaf_nodes == self.max_leaf_nodes):
             self._finalize_leaf(left_child_node)
             self._finalize_leaf(right_child_node)
+            self._finalize_splittable_nodes()
             return left_child_node, right_child_node
 
-        if (self.max_leaf_nodes is not None
-                and n_leaf_nodes == self.max_leaf_nodes):
+        if self.max_depth is not None and depth == self.max_depth:
             self._finalize_leaf(left_child_node)
             self._finalize_leaf(right_child_node)
-            self._finalize_splittable_nodes()
             return left_child_node, right_child_node
 
         if left_child_node.n_samples < self.min_samples_leaf * 2:

sklearn/ensemble/_hist_gradient_boosting/tests/test_gradient_boosting.py

@@ -445,3 +445,17 @@ def test_string_target_early_stopping(scoring):
     y = np.array(['x'] * 50 + ['y'] * 50, dtype=object)
     gbrt = HistGradientBoostingClassifier(n_iter_no_change=10, scoring=scoring)
     gbrt.fit(X, y)
+
+
+def test_max_depth_max_leaf_nodes():
+    # Non regression test for
+    # https://github.com/scikit-learn/scikit-learn/issues/16179
+    # there was a bug when the max_depth and the max_leaf_nodes criteria were
+    # met at the same time, which would lead to max_leaf_nodes not being
+    # respected.
+    X, y = make_classification(random_state=0)
+    est = HistGradientBoostingClassifier(max_depth=2, max_leaf_nodes=3,
+                                         max_iter=1).fit(X, y)
+    tree = est._predictors[0][0]
+    assert tree.get_max_depth() == 2
+    assert tree.get_n_leaf_nodes() == 3  # would be 4 prior to bug fix