Repository: emlearn/emlearn (language: python)

Issue:
trees: Separate out leaves - prep for proba output and soft voting
This moves the leaves out of the nodes and into a separate array, such that they can have different sizes depending on the desired output. For soft-voting one needs to store class probabilities. Preparation for fixing #12 

DONE
* [x] Move leaves out of tree
* [x] Add test for soft-voting case. 
* [x] Add tests for predict_proba()

TODO
* [ ] Benchmark to check that performance did not regress (inference time and model size)
* [ ] Make soft-voting the default behavior (for maximum compatibility)
* [ ] Add documentation on leaf_bits / hard voting.
* [ ] And example of using leaf_bits


Discussion / hints:

Fix patch:
diff --git a/emlearn/trees.py b/emlearn/trees.py
index 8ba378b..41b5e80 100644
--- a/emlearn/trees.py
+++ b/emlearn/trees.py
@@ -202,10 +202,26 @@ def forest_to_dot(forest, name='trees', indent="  "):
 
 
 def generate_c_nodes(flat, name):
-    def node(n):
-        return "{{ {}, {}, {}, {} }}".format(*n)
+    child_value_max = 2**15
+    child_value_min = -2**15
 
-    nodes_structs = ',\n  '.join(node(n) for n in flat)
+    def assert_valid_child(value):
+        assert value >= child_value_min, value
+        assert value <= child_value_max, value
+
+    def make_node(index, node):
+        feature, value, left_index, right_index = node
+
+        # XXX: consider using relative jumps?
+        left = left_index
+        right = right_index
+    
+        assert_valid_child(left)
+        assert_valid_child(right)
+
+        return "{{ {}, {}, {}, {} }}".format(feature, value, left, right)
+
+    nodes_structs = ',\n  '.join(make_node(i, n) for i, n in enumerate(flat))
     nodes_name = name
     nodes_length = len(flat)
     nodes = "EmlTreesNode {nodes_name}[{nodes_length}] = {{\n  {nodes_structs} \n}};".format(**locals());
@@ -378,6 +394,11 @@ def __init__(self, estimator, classifier, dtype='float'):
         self.forest_ = flatten_forest(trees, leaf=leaf)
         self.forest_ = remove_duplicate_leaves(self.forest_)
 
+        n_nodes = len(self.forest_[0])
+        max_nodes = 2**15 # limited by int16_t for children in EmlTreeNode structure
+        if n_nodes > max_nodes:
+            raise ValueError(f"Model has {n_nodes} nodes. Max supported is {max_nodes} nodes.")
+
         if classifier == 'pymodule':
             # FIXME: use Nodes,Roots directly, as Numpy Array
             import eml_trees # import when required


Test patch:
diff --git a/test/test_trees.py b/test/test_trees.py
index 0cb53a4..7e9c921 100644
--- a/test/test_trees.py
+++ b/test/test_trees.py
@@ -9,6 +9,7 @@
 from sklearn.tree import DecisionTreeClassifier, DecisionTreeRegressor
 
 import emlearn
+from emlearn.evaluate.trees import model_size_nodes
 import pytest
 
 random = numpy.random.randint(0, 1000)
@@ -68,6 +69,20 @@ def test_trees_sklearn_regressor_predict(data, model, method):
     numpy.testing.assert_allclose(pred_c, pred_original, rtol=1e-3, atol=2)
 
 
+def test_trees_too_many_nodes():
+    """should give nice error"""
+    X, Y = datasets.make_classification(n_classes=2, n_samples=1000, random_state=1)
+    estimator = RandomForestClassifier(n_estimators=1000, max_depth=20, random_state=1)
+    estimator.fit(X, Y)
+
+    with pytest.raises(ValueError) as ex:
+        cmodel = emlearn.convert(estimator, method='loadable')
+
+    error = str(ex.value).lower()
+    assert 'nodes' in error
+    assert 'model has ' in error
+    assert 'max supported ' in error
+
 def test_deduplicate_single_tree():
     nodes = [
         [ -1, 1, -1, -1 ],