[slievens]

Fixes #524

Checklist

• My branch is up-to-date with main branch.
• Everything works and tested on latest stable Rust.
• Coverage and Linting have been applied

Current behaviour

Extra trees (Splitter::Random) use the same data layout as random forests (Splitter::Best):

• The forest sorts every feature column once (argsort).
• Each tree keeps one sorted index list for each feature.
• After each split, the tree partitions all of these lists.

Extra trees do not need sorted data. They draw a random threshold between the minimum and the maximum of the feature. Thus the sort and the partition of the p lists are extra work. This work increases with the number of features.

New expected behaviour

For Splitter::Random:

• The forest does not sort the columns. order is now Option<&[Vec<usize>]> and is None for extra trees.
• Each tree keeps one unsorted index list (columns[0]). After a split, the tree partitions only this list.
• find_random_split gathers the values of the feature and the targets of the node into reusable buffers. It then gets the minimum and the maximum in one pass, and counts the true branch without a sorted early exit.
• split assigns each row to the true or false branch in one pass over the unsorted list.

For Splitter::Best (random forests, decision trees) there is no change in behaviour. A presorted flag in SplitWorkspace selects the old code path.

The predictions of extra trees are not bit-identical to main, but they have the same statistical behaviour. The sklearn parity tests of ExtraTreesRegressor pass with no change in reference values or tolerances.

Changed

• Extra trees do not sort the feature columns.
• BaseTree fit functions take order: Option<&[Vec<usize>]> (crate internal)

[codecov]

Codecov Report

❌ Patch coverage is 98.69281% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.98%. Comparing base (9eaae9e) to head (8b6482a).
⚠️ Report is 222 commits behind head on main.

Files with missing lines	Patch %	Lines
src/tree/base_tree.rs	98.62%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #525       +/-   ##
===========================================
+ Coverage   43.97%   86.98%   +43.00%     
===========================================
  Files          85       99       +14     
  Lines        7281     9898     +2617     
===========================================
+ Hits         3202     8610     +5408     
+ Misses       4079     1288     -2791     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Mec-iS]

how big can be this matrix? please be considerate with allocation, it would be better to use DenseMatrix here so that we keep the zero-copy patterns.

[slievens]

this was here before this PR. This contains the sorted NodeElements. These are kept as small as possible through the use of different implementations UnitElement, CountedElement, WeightedElement.

In the case of "Best" it contains the same number of rows and columns as X, but it does not contain the actual elements of x.

[Mec-iS]

Thanks for this. The approach matches #524, and the Splitter::Random path is now O(p) lighter in both memory and partition work. Review notes, focused on allocation and loops:

1. Possible allocation regression on the Splitter::Best path (split)
let mut true_stats = criterion.empty_stats(); is now created before the if workspace.presorted branch, and the presorted branch then overwrites it with true_stats = parent_stats.clone();. If Stats owns a Vec (for example, class counts), this is one extra allocation per split node for random forests and decision trees. This conflicts with the claim of no change for Splitter::Best. Use deferred initialization so each branch creates it once: let mut true_stats; then true_stats = parent_stats.clone(); in the presorted branch and true_stats = criterion.empty_stats(); in the other.

2. Pre-size the single column
In the None arm, counts.iter().enumerate().filter(..).map(..).collect() has no exact size hint, so the Vec grows by doubling. n_kept is already computed just above, so use Vec::with_capacity(n_kept) and push in a loop (or extend). That gives one allocation of the exact size.

3. Per-node work in split (Random branch)

• criterion.target(*visitor.y.get(e.row())) is evaluated two or three times per element. Bind it once with let t = ....
• workspace.node_targets already holds the targets of this node in the same order as columns[0][start..end]. If split always runs right after the search for the same node, reuse it and skip the y lookups. Please confirm the invariant, or add an assert_eq! on the lengths.
• Each element does three criterion.add calls (parent, true, false). Only two should be needed: add to the true and false stats, then derive the parent (there is already a difference operation). If Criterion has no merge, ignore this.

4. Unused partition buffer and an unfinished note
The comment this can actually be an "unstable partition" is a TODO in disguise, and the project rule is to open an issue instead. For the unsorted list an in-place swap partition needs no partition_buffer and is faster, so it fits this PR. Either implement it or open an issue and remove the comment.

5. One source of truth for the mode
presorted comes from order.is_some(), but random_buffer_len comes from parameters.splitter. Splitter::Best with order = None would index columns[j] out of bounds for j > 0, and Splitter::Random with Some(order) silently takes the sorted path. Derive both from one value, or check order.is_some() == matches!(splitter, Splitter::Best) once at the top and return Failed otherwise.

6. Layout of Vec<Vec<E>> (follow-up, not required here)
For Splitter::Best, columns and the forest-level order are p separate heap allocations per tree and per forest. A flat Vec<E> of length p * n_kept, sliced by j * n_kept, is one allocation with better locality. The Random case is the same layout with p = 1, which would also remove the columns[0] convention. This is worth a separate issue.

7. Loop costs

• split (presorted) still does p stable partitions per node, so O(p * n_node). This is unchanged and fine for this PR.
• If workspace.variables is shuffled in full at every node, that is O(p) per node, which now matters more because the sort and the p partitions are gone. A partial Fisher-Yates shuffle of the first mtry entries is enough. Please check the current code.
• The loops in find_random_split use col.get(e.row()). The project guideline says to access data through iterator(...). For a row-major matrix this is a strided read, so one gather per feature (as done here) is right. Please note that choice in the PR.
• zip stops silently at the shortest input. Add debug_assert_eq! on n_node, node_targets.len() and the slice length.

8. Tests and evidence

• The diff only changes signatures in tests. Please add tests for the new path: fit with order = None for a node with a constant feature, bootstrap with duplicate rows, sample weights, and min_samples_leaf at the boundary.
• Add benchmark numbers (fit time and peak memory before and after) for a few sizes, for example 1k x 10, 10k x 50 and 100k x 20, since the PR is about performance.

9. Small points

• The CHANGELOG line reads not bit-identical the old ones; it should be to the old ones.

[slievens]

I addressed all of the important points above in the latest commit. These are the results of a hyperfine benchmark run.

Extra trees fit: main (bb7c794) vs. this branch (6ba32a7)

ExtraTreesRegressor, max_depth = 10, no bootstrap. Mean of 5 runs (1 warmup) with hyperfine.
Speedup = main / branch (> 1 means that the branch is faster).

rows	features	trees	main [s]	branch [s]	speedup
1000	50	100	0.054 ± 0.001	0.043 ± 0.000	1.25
1000	50	200	0.103 ± 0.003	0.086 ± 0.001	1.20
1000	50	500	0.254 ± 0.002	0.215 ± 0.004	1.18
1000	100	100	0.103 ± 0.003	0.061 ± 0.002	1.68
1000	100	200	0.198 ± 0.004	0.118 ± 0.002	1.68
1000	100	500	0.482 ± 0.005	0.296 ± 0.005	1.63
1000	200	100	0.211 ± 0.007	0.096 ± 0.002	2.21
1000	200	200	0.402 ± 0.019	0.176 ± 0.003	2.28
1000	200	500	0.992 ± 0.028	0.451 ± 0.013	2.20
5000	50	100	0.260 ± 0.012	0.196 ± 0.003	1.33
5000	50	200	0.518 ± 0.020	0.383 ± 0.010	1.35
5000	50	500	1.245 ± 0.045	0.944 ± 0.019	1.32
5000	100	100	0.496 ± 0.019	0.294 ± 0.010	1.68
5000	100	200	0.970 ± 0.035	0.590 ± 0.010	1.64
5000	100	500	2.362 ± 0.065	1.468 ± 0.021	1.61
5000	200	100	0.946 ± 0.033	0.443 ± 0.037	2.14
5000	200	200	1.889 ± 0.071	0.878 ± 0.071	2.15
5000	200	500	4.703 ± 0.186	2.115 ± 0.067	2.22
10000	50	100	0.529 ± 0.007	0.408 ± 0.013	1.30
10000	50	200	1.070 ± 0.040	0.850 ± 0.028	1.26
10000	50	500	2.520 ± 0.080	2.047 ± 0.075	1.23
10000	100	100	0.991 ± 0.039	0.612 ± 0.038	1.62
10000	100	200	1.999 ± 0.121	1.195 ± 0.040	1.67
10000	100	500	4.940 ± 0.239	3.044 ± 0.166	1.62
10000	200	100	1.846 ± 0.085	0.832 ± 0.010	2.22
10000	200	200	3.771 ± 0.710	1.622 ± 0.029	2.33
10000	200	500	9.507 ± 0.732	4.060 ± 0.357	2.34

Geometric mean speedup: 1.67 (27 cases). The speedup increases with the number of
features: about 1.25× for 50 features, 1.65× for 100 and 2.2× for 200.

The fitted forests are different from main, because ties between equal splits can be
broken in a different way. The test MSE stays the same (for example 23.170 vs. 23.171 for
10000 rows, 200 features, 500 trees).

The values after "±" are the standard deviations that hyperfine reported.