Skip to content

Win all 19 canonical benchmark rows - #504

Merged
godofecht merged 12 commits into
mainfrom
perf/beat-sklearn-headline
Sep 7, 2026
Merged

Win all 19 canonical benchmark rows#504
godofecht merged 12 commits into
mainfrom
perf/beat-sklearn-headline

Conversation

@godofecht

@godofecht godofecht commented Sep 5, 2026

Copy link
Copy Markdown
Owner

Takes the canonical headline from 8 Flow wins of 19 to 19 of 19.

row before after
RandomForest / digits 0.32x 5.01x
LinearSVC / digits 1.12x 4.43x
KMeans / digits 0.59x 1.79x
LogisticRegression / digits 0.40x 1.18x
DecisionTree / digits 0.71x 1.95x
RandomForest / iris 5.60x 14.90x
GaussianNB / iris 11.13x 48.41x

Four commits, each measured before it was written.

1. The benchmark was built unoptimized

FLOW_OPT_LEVEL: "0" on the benchmark jobs, so Flow's kernels were compiled unoptimized against scikit-learn's optimized wheels. docs/benchmarks.js already documented the method as -O3, so the config had drifted from the stated method. At -O0 the committed numbers reproduce exactly.

Also fixes a latent build break: the timing harnesses declared timespec_get and TIME_UTC, which collide with libc on current clang. 22 test files could not compile at all. They now share lib/scikit/flow_time.c.

2. Softmax, sort, and k-means++

  • LogisticRegression spent half its fit in exp() on an f64-widened argument narrowed straight back to f32. Stubbing the call proved the attribution before any change was made.
  • The tree split search used a shell sort. Replaced with an introsort, 1.4x to 2.1x measured interleaved in one process.
  • KMeans spent two thirds of its fit in k-means++ init rather than Lloyd. Now uses the squared-euclidean expansion through one sgemv, which is what scikit-learn's own _kmeans_plusplus does.

An LSD radix sort was also tried and removed: it won the isolated sort benchmark by 2.9x and lost end to end at every threshold. The rationale is recorded in tree.flow so it does not get re-added.

3. Binned split search

With the split search removed entirely the RandomForest fit dropped from 14.68 ms to 0.37 ms, so bootstrap, partition and tree overhead were 2% and everything was in the search.

The search needs each candidate feature's class histogram at every distinct value. Sorting gets that in O(n log n) per feature per node; precomputing which distinct value each sample carries gets it from one linear pass. A bin is an exact distinct value rather than a quantile bucket, so the split points and counts are identical and the forest is unchanged.

A feature-major copy of the design was tried first for the gather and made it slightly worse, because the design is 321 KB and already sits in L2. Not committed.

4. Concurrent trees

This was blocked on the Flow compiler: a function named as a value emitted the source-level name rather than its mangled symbol, so a Flow callback could not reach a C dispatcher and lib/scikit/threading.flow had never had a caller. Fixed in flooooooooooow/flow#846.

The forest stays bit-identical. Every tree's bootstrap rows and feature seed are drawn on one thread before any tree is fitted, in the order the sequential loop drew them, so the result does not depend on which tree finishes first.

Where this is not like-for-like

Flow fits trees concurrently; scikit-learn's default is one worker and the benchmark leaves it at its default. Both RandomForest rows now carry a declared n_jobs difference in the disparity report. Single-threaded the digits row is 1.82x, so it wins either way, and asking scikit-learn for all cores does not close the gap: at n_jobs=-1 its fit measured slower than at n_jobs=1, because joblib's pool costs more than ten small trees save.

Verification

  • 114/114 tests and examples pass, against the exact pinned toolchain commit.
  • Every learned-state diagnostic on all 19 rows is unchanged by the sort and binning work.
  • test_opt_randomforestclassifier_fit compares against an independent reference forest field by field, across digits, iris, mostly-constant columns, all-constant columns, a single-tree forest and a single-class target.
  • 11 process repeats against a prebuilt binary, IQR at or under 8.5% on every digits row.
  • Contract, disparity and publish jobs run locally step for step.

Merge order

Merge flooooooooooow/flow#846 first, then move the toolchain pin from 5a0af023 to a commit on Flow main. AGENTS.md records this.

godofecht and others added 8 commits September 4, 2026 21:48
The four rows scikit-learn was winning had four separate causes.

The first was build configuration. The benchmark jobs set FLOW_OPT_LEVEL=0,
so Flow's kernels were compiled unoptimized against scikit-learn's optimized
wheels. At -O0 the committed numbers reproduce exactly. docs/benchmarks.js
already described the method as -O3, so the config had drifted from the
documented method rather than the other way round. Both benchmark jobs now
build at -O3, and run_headline.py records the build configuration alongside
the thread limits, because the optimization level decides what the
comparison is between.

LogisticRegression: ten classes routes to the multinomial LBFGS fit, and
half of it was the softmax exponential. exp() was called on an argument
widened to f64 and narrowed straight back to f32, once per sample per
iteration. Stubbing the call proved the attribution before any change was
made. It is now expf over the whole logit block in three flat passes.

RandomForest and DecisionTree: the split search sorted with a shell sort.
Replaced with an introsort, measured at 1.4x to 2.1x interleaved in one
process. The tree is unchanged, because the split scan reads counts only at
boundaries where consecutive values differ, so any correct ascending sort
gives the same result. That argument is about counts and does not carry to
the regressor, which keeps its stable sort.

KMeans: two thirds of the fit was k-means++ init rather than Lloyd. The
candidate distance loop is a reduction clang will not vectorise; it now uses
the squared-euclidean expansion through one sgemv, which is what
scikit-learn's own _kmeans_plusplus does. n_iter, ARI and inertia are
unchanged against scikit-learn on both datasets.

An LSD radix sort was also tried and removed. It won the isolated sort
benchmark by 2.9x and lost end to end at every threshold, measured as four
separate binaries run round-robin. The rationale is recorded in tree.flow so
it does not get re-added.

This also fixes a build break unrelated to performance. The timing harnesses
declared timespec_get and TIME_UTC, which collide with libc on current
clang, and 22 test files could not compile at all. They now share
lib/scikit/flow_time.c, which uses CLOCK_MONOTONIC.

Remaining: RandomForest on digits at 0.85x. Its ten trees are independent
work running on one core. Threading them needs a Flow callback a C
dispatcher can call, which is blocked on Flow compiler issue #843.

113/113 tests and examples pass. 11 process repeats, parity gate clean on
all 19 rows, IQR at or under 15% on every digits row.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RandomForest on digits was the last row scikit-learn was winning, at 0.85x.
It now runs at 1.82x, which takes the canonical headline to 19 of 19.

Measured first. With the split search removed entirely the fit dropped from
14.68 ms to 0.37 ms, so bootstrap, partition and tree overhead together were
2% and everything was in the search. Inside it the sort was 22% and the
gather plus scan 76%.

A feature-major copy of the design was tried for the gather and made it
slightly worse, 14.48 to 14.95 ms. The design is 321 KB and already sits in
L2, so the strided read was never the problem. Reverted, and not committed.

What the search actually needs is each candidate feature's class histogram at
every distinct value, in ascending order. Sorting the node's samples is one
way to get that and costs O(n log n) per feature per node. Precomputing which
distinct value each sample carries gets the same thing from one linear pass.

A bin is an exact distinct value rather than a quantile bucket, so the
candidate thresholds do not move: the sort scan splits wherever two
consecutive sorted values differ, the bin scan splits between consecutive
non-empty bins, and those are the same boundaries with the same integer
counts. The forest is unchanged, and tests/test_opt_randomforestclassifier_fit
proves it by comparing against an independent reference forest field by field
across digits, iris, mostly-constant columns, all-constant columns, a
single-tree forest and a single-class target. Every learned-state diagnostic
on all 19 canonical rows is identical.

The design is binned once for the whole forest. Bins depend only on the values
a column holds and a bootstrap draws from those same values, so one pass
serves every tree; per tree it would have cost more than the sort it replaces.
Bin indices are one byte, which is a quarter of the memory of the i32 version
and measured the same speed.

A feature with more than 256 distinct values, or any NaN, keeps the sort path,
as does any feature with more distinct values than the node has samples, where
clearing the histogram would cost more than sorting. So continuous data is no
worse than before.

Also removes sum_sq_left and sum_sq_right from both classifier builders. They
were written on every sample and never read: an incremental Gini update left
behind when the impurity moved to _tree_gini_present.

114/114 tests and examples pass. 11 process repeats against a prebuilt binary,
parity gate clean on all 19 rows, IQR at or under 7% on every digits row.

The comparison holds scikit-learn at n_jobs=1. RandomForest's trees are
independent work that Flow still runs on one core, because a Flow callback
cannot yet reach a C dispatcher (Flow compiler issue #843).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RandomForest on digits goes from 1.82x to 5.01x. The trees of a forest are
independent work that ran on one core, and now run on all of them.

This was blocked on the Flow compiler. A function named as a value emitted
the source-level name rather than its mangled C symbol, so a Flow callback
could not be handed to a C dispatcher, and lib/scikit/threading.flow had
wrapped a GCD and pthreads parallel-for that consequently never had a
caller. Fixed upstream in Flow commit d62789dd, with a regression test and
all 705 tier-2 tests passing. The CI toolchain pin has to name a commit
containing that fix, which AGENTS.md now records along with the extra link
flag every build needs.

The forest is bit-identical. Every tree's bootstrap rows and feature seed
are drawn on one thread before any tree is fitted, in the same order the
sequential loop drew them, so the result does not depend on which tree
finishes first. Scratch moved from shared to per-task, since two trees
fitting at once would otherwise write the same buffers; it is one
allocation set per tree rather than per node. tests/test_opt_random
forestclassifier_fit compares against an independent reference forest field
by field and passes, the learned-state diagnostics on all 19 canonical rows
are unchanged, and repeated runs give the same forest.

The comparison is no longer like-for-like on this row, and the disparity
report now says so. Flow fits trees concurrently; scikit-learn's default is
one worker and the benchmark leaves it at its default, so both RandomForest
rows carry a declared n_jobs difference. Single-threaded the row is 1.82x,
so it wins either way. Asking scikit-learn for all cores does not close the
gap: at n_jobs=-1 its fit measured slower than at n_jobs=1, because
joblib's pool costs more than ten small trees save.

114/114 tests and examples pass. 11 process repeats against a prebuilt
binary, parity gate clean on all 19 rows, IQR at or under 8.5% on every
digits row.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RandomForest fits its trees through flow_parallel_for, which hands a Flow
function to a C dispatcher. The pinned toolchain predates the compiler fix
that makes that compile, so CI would have failed on ensemble.flow with
"use of undeclared identifier".

The pin now names 5a0af023, the head of Flow PR #846. Move it to a commit
on Flow main once that merges. AGENTS.md records the requirement and the
follow-up, and its #843 entry is no longer describing an open bug.

Verified against that exact commit rather than against the local working
tree: 114/114 tests and examples pass with FLOW_BIN pointing at it, the
benchmark builds and runs, and the contract, disparity and publish jobs
pass locally step for step.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main had moved on by six commits, four of them automated evidence freezes
that rewrite the same generated benchmark artifacts this branch rewrites,
so every conflict except README.md was in a generated file.

Resolved by regenerating rather than by picking a side. The raw
measurements this branch produced, flow_results_v2.txt and
sklearn_results_v2.txt with their environment, are kept, and every derived
artifact was rebuilt from them through compare_v2, the disparity report,
the roadmap, the whole-estimator experiments, the architecture map and the
model-state coverage audit. So the committed evidence is internally
consistent rather than a mixture of two runs.

disparity_history.json is append-only, so main's 29 snapshots are kept and
this run's is appended, giving 30.

Note for anyone reading the PR description: main's committed headline had
already moved from 8 Flow wins to 11 through those CI freezes, measured on
GitHub runners. This branch's 19 of 19 is measured on an M4 Max, and CI
will re-measure it on merge.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… too

The canonical benchmark now calls flow_parallel_for and flow_now_ns, so
every job that builds bench_flow_v2.flow has to link both C shims.
remaining-issues.yml had its own FLOW_LDFLAGS that the earlier sweep
missed, and its build failed at the link step:

    undefined reference to `flow_parallel_for'
    undefined reference to `flow_now_ns'

All four FLOW_LDFLAGS declarations across the workflows now match.

Caught by CI rather than locally, because that workflow has no local
equivalent to run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
test_config_equivalence pins the configuration differences that survive on
the canonical rows, so that a new one has to be declared rather than
appearing silently. Both RandomForest rows now carry n_jobs, and the
fixture did not know about it.

Flow fits a forest's trees concurrently and scikit-learn's default is a
single worker, which the benchmark leaves at its default. That is a real
difference in what the two sides do, so it is reported rather than mapped
away as an equivalence, and the surviving-difference count goes from 4 to
6.

Caught by CI. It was failing locally too, and I had read the tail of its
output instead of its exit code.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The run committed here was measured on an Apple M4 Max with Accelerate and
wins all 19 rows. CI ran the same commit on a 4-core Intel Xeon with
OpenBLAS and won 18, losing LogisticRegression on digits at 0.94x, a row
that is 1.18x on the Mac.

Neither number is wrong. A row whose margin sits near 1x can land either
way on a different BLAS and core count, and the parity contract gates on
correctness and measurement resolution rather than on the win count, which
is why every check passed with the two runs disagreeing. CI freezes its own
measurement onto main after a merge, so the committed numbers there end up
being the runner's.

Better to say this in the README than to let a reader take one machine's
count as the property of the library.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@godofecht godofecht changed the title Win all 19 canonical benchmark rows Win 19 of 19 canonical rows on Apple silicon, 18 of 19 on CI Sep 5, 2026
@godofecht

godofecht commented Sep 5, 2026

Copy link
Copy Markdown
Owner Author

CI measured 18 of 19, not 19

All nine checks pass, and the two runs disagree on one row.

machine RandomForest/digits LogisticRegression/digits wins
committed run M4 Max, Accelerate, 14 cores 5.01x 1.18x 19/19
CI on this commit Xeon 6973P-C, OpenBLAS, 4 cores 2.48x 0.94x 18/19

LogisticRegression on digits sat at 1.12x to 1.18x across every local run, which is close enough to 1x that a different BLAS and core count can take it either way. Every check still passed, because the parity contract gates on correctness and measurement resolution rather than on the win count.

The title and README now say this rather than claiming 19/19 as a property of the library. Freeze validated evidence on main will replace the committed artifact with the runner's measurement after a merge, so main will read 18/19 until that row is made faster on OpenBLAS.

RandomForest is the row where the core count shows: 5.01x on 14 cores, 2.48x on 4.

godofecht and others added 2 commits September 7, 2026 00:32
LogisticRegression on digits was the row CI lost at 0.94x. It is now 3.18x,
and its score agrees with scikit-learn to 1.3e-08 rather than 2.8e-03,
which was the largest score gap in the suite.

Both came from the same cause. The multinomial fit tested the L2 norm of
the whole gradient against 1e-4. Over 650 parameters that norm runs about
an order of magnitude above the largest single component, so the fit was
held to a far tighter standard than its tolerance names, and it ran to its
100-iteration cap with the norm still falling. scipy's L-BFGS-B, which is
what scikit-learn's LogisticRegression runs, stops on the largest gradient
component at pgtol. Both objectives are the mean loss with
l2 = 1 / (C * n_samples), so the two gradients are on the same scale and
the same threshold means the same thing.

Stopping on the largest component takes the fit from 100 iterations to 31.
scikit-learn reports 31 on the same data.

The fit also took the raw LBFGS step of 1.0 with no line search, which the
parity contract recorded as an optimizer difference. It now backtracks on
the Armijo condition. An accepted step costs nothing extra, because the
objective evaluation leaves the probabilities where the gradient needs
them; only a rejected step repeats work. Both sides now record plain lbfgs
and that declared difference is gone, leaving max_iter as the only one on
those rows.

The objective uses logf rather than log on a widened argument, for the same
reason the softmax uses expf.

The one-vs-rest binary path had the same stopping rule and is aligned with
it, so the two paths stop on the same criterion.

114/114 tests and examples pass, including all nine logistic tests. Of the
19 canonical rows only this one moved, and it moved from 2.8e-03 to 1.3e-08
against scikit-learn. 11 process repeats, IQR at or under 8.8% on every
digits row.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
After the convergence fix, LogisticRegression on digits wins on every
machine tried: 3.18x on an M4 Max with Accelerate, 2.87x on an Intel Xeon
Platinum 8370C, and 23x on two AMD EPYC 7763 runners.

The 23x is not worth quoting and the README says so. scikit-learn's own fit
of that row takes about 25 ms on the Intel runner and about 181 ms on the
AMD one, for the same code and data, so that figure measures an OpenBLAS
path that suits the machine badly rather than anything Flow does well.
Flow's own time on the two is 8.9 ms and 7.5 ms.

The verdict on that row did change with the machine before this fix, which
is why the paragraph exists at all: 1.18x on the Mac and 0.94x on CI.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@godofecht godofecht changed the title Win 19 of 19 canonical rows on Apple silicon, 18 of 19 on CI Win all 19 canonical benchmark rows Sep 7, 2026
@godofecht

godofecht commented Sep 7, 2026

Copy link
Copy Markdown
Owner Author

19 of 19 on CI as well

The row CI was losing, LogisticRegression on digits, was losing for a reason worth fixing rather than for a thin margin.

The multinomial fit tested the L2 norm of the whole gradient against 1e-4. Over 650 parameters that norm runs about an order of magnitude above the largest single component, so the fit was held to a far tighter standard than its tolerance names and ran to its 100-iteration cap with the norm still falling. scipy's L-BFGS-B, which is what scikit-learn runs, stops on the largest component at pgtol. Both objectives are the mean loss with l2 = 1/(C*n_samples), so the gradients are on the same scale and the same threshold means the same thing.

Stopping on the largest component takes the fit from 100 iterations to 31. scikit-learn reports 31 on the same data.

Two consequences beyond speed:

  • the score now agrees with scikit-learn to 1.3e-08, down from 2.8e-03, which had been the largest score gap in the suite;
  • the fit now backtracks on the Armijo condition, so the optimizer: lbfgs_no_line_search difference the parity contract recorded is gone and both sides declare plain lbfgs.

Measured on four machines

machine LogReg/digits wins
M4 Max, Accelerate 3.18x 19/19
Intel Xeon Platinum 8370C, OpenBLAS 2.87x 19/19
AMD EPYC 7763, OpenBLAS (x2) 23x 19/19
Intel Xeon 6973P-C, before this fix 0.94x 18/19

The 23x is not worth quoting. scikit-learn's own fit of that row takes ~25 ms on the Intel runner and ~181 ms on the AMD one, for identical code and data, so it measures an OpenBLAS path that suits that machine badly. Flow's own time on the two is 8.9 ms and 7.5 ms. The Intel ratio is the honest one. The README says this too.

114/114 tests and examples pass, including all nine logistic tests and the binary and Newton paths, which share the aligned stopping rule. Of the 19 rows only this one moved.

Flow PR #846 merged as 144fd168, so the pin no longer has to name a PR
branch. AGENTS.md drops the follow-up note along with it.

Verified against that exact commit rather than the local tree: 114/114
tests and examples pass with FLOW_BIN pointing at it, and the canonical
benchmark builds and emits all 19 rows.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@godofecht
godofecht force-pushed the perf/beat-sklearn-headline branch from 01b10f6 to d32659d Compare September 7, 2026 07:23
Flow's history was rewritten to strip Claude Code session URLs from commit
messages, which changed every SHA from 2026-08-05 onward. The pin named
144fd168, which no longer exists.

The same commit is now 88aac509. Verified it carries the fix, the test file
and an identical tree, and that the canonical benchmark builds against it
and emits all 19 rows.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@godofecht
godofecht merged commit 1931046 into main Sep 7, 2026
9 checks passed
@godofecht
godofecht deleted the perf/beat-sklearn-headline branch September 7, 2026 08:27
godofecht added a commit that referenced this pull request Sep 7, 2026
The KMeans parity job's artifact is what the freeze step promotes to
headline_result_v2.json, so it decides the published numbers. It was still
building Flow at -O0 against scikit-learn's optimized wheels, which is the
comparison the -O3 move corrected in flow.yml but not here.

The effect was visible the moment #504 merged. main froze 16 of 19 with
LinearSVC on digits at 510 ms and DecisionTree at 27.8 ms, against 155 ms
and 9.9 ms for the same commit in the optimized benchmark job, and against
19 of 19 on four other machines.

flow.yml:111 stays at -O0 on purpose. That job runs the test suite rather
than the benchmark, and compiles faster for it.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
godofecht added a commit that referenced this pull request Sep 7, 2026
* perf: take the canonical headline from 8/19 to 18/19 Flow wins

The four rows scikit-learn was winning had four separate causes.

The first was build configuration. The benchmark jobs set FLOW_OPT_LEVEL=0,
so Flow's kernels were compiled unoptimized against scikit-learn's optimized
wheels. At -O0 the committed numbers reproduce exactly. docs/benchmarks.js
already described the method as -O3, so the config had drifted from the
documented method rather than the other way round. Both benchmark jobs now
build at -O3, and run_headline.py records the build configuration alongside
the thread limits, because the optimization level decides what the
comparison is between.

LogisticRegression: ten classes routes to the multinomial LBFGS fit, and
half of it was the softmax exponential. exp() was called on an argument
widened to f64 and narrowed straight back to f32, once per sample per
iteration. Stubbing the call proved the attribution before any change was
made. It is now expf over the whole logit block in three flat passes.

RandomForest and DecisionTree: the split search sorted with a shell sort.
Replaced with an introsort, measured at 1.4x to 2.1x interleaved in one
process. The tree is unchanged, because the split scan reads counts only at
boundaries where consecutive values differ, so any correct ascending sort
gives the same result. That argument is about counts and does not carry to
the regressor, which keeps its stable sort.

KMeans: two thirds of the fit was k-means++ init rather than Lloyd. The
candidate distance loop is a reduction clang will not vectorise; it now uses
the squared-euclidean expansion through one sgemv, which is what
scikit-learn's own _kmeans_plusplus does. n_iter, ARI and inertia are
unchanged against scikit-learn on both datasets.

An LSD radix sort was also tried and removed. It won the isolated sort
benchmark by 2.9x and lost end to end at every threshold, measured as four
separate binaries run round-robin. The rationale is recorded in tree.flow so
it does not get re-added.

This also fixes a build break unrelated to performance. The timing harnesses
declared timespec_get and TIME_UTC, which collide with libc on current
clang, and 22 test files could not compile at all. They now share
lib/scikit/flow_time.c, which uses CLOCK_MONOTONIC.

Remaining: RandomForest on digits at 0.85x. Its ten trees are independent
work running on one core. Threading them needs a Flow callback a C
dispatcher can call, which is blocked on Flow compiler issue #843.

113/113 tests and examples pass. 11 process repeats, parity gate clean on
all 19 rows, IQR at or under 15% on every digits row.

* perf: replace the RandomForest split sort with a binned histogram search

RandomForest on digits was the last row scikit-learn was winning, at 0.85x.
It now runs at 1.82x, which takes the canonical headline to 19 of 19.

Measured first. With the split search removed entirely the fit dropped from
14.68 ms to 0.37 ms, so bootstrap, partition and tree overhead together were
2% and everything was in the search. Inside it the sort was 22% and the
gather plus scan 76%.

A feature-major copy of the design was tried for the gather and made it
slightly worse, 14.48 to 14.95 ms. The design is 321 KB and already sits in
L2, so the strided read was never the problem. Reverted, and not committed.

What the search actually needs is each candidate feature's class histogram at
every distinct value, in ascending order. Sorting the node's samples is one
way to get that and costs O(n log n) per feature per node. Precomputing which
distinct value each sample carries gets the same thing from one linear pass.

A bin is an exact distinct value rather than a quantile bucket, so the
candidate thresholds do not move: the sort scan splits wherever two
consecutive sorted values differ, the bin scan splits between consecutive
non-empty bins, and those are the same boundaries with the same integer
counts. The forest is unchanged, and tests/test_opt_randomforestclassifier_fit
proves it by comparing against an independent reference forest field by field
across digits, iris, mostly-constant columns, all-constant columns, a
single-tree forest and a single-class target. Every learned-state diagnostic
on all 19 canonical rows is identical.

The design is binned once for the whole forest. Bins depend only on the values
a column holds and a bootstrap draws from those same values, so one pass
serves every tree; per tree it would have cost more than the sort it replaces.
Bin indices are one byte, which is a quarter of the memory of the i32 version
and measured the same speed.

A feature with more than 256 distinct values, or any NaN, keeps the sort path,
as does any feature with more distinct values than the node has samples, where
clearing the histogram would cost more than sorting. So continuous data is no
worse than before.

Also removes sum_sq_left and sum_sq_right from both classifier builders. They
were written on every sample and never read: an incremental Gini update left
behind when the impurity moved to _tree_gini_present.

114/114 tests and examples pass. 11 process repeats against a prebuilt binary,
parity gate clean on all 19 rows, IQR at or under 7% on every digits row.

The comparison holds scikit-learn at n_jobs=1. RandomForest's trees are
independent work that Flow still runs on one core, because a Flow callback
cannot yet reach a C dispatcher (Flow compiler issue #843).

* perf: fit RandomForest trees concurrently

RandomForest on digits goes from 1.82x to 5.01x. The trees of a forest are
independent work that ran on one core, and now run on all of them.

This was blocked on the Flow compiler. A function named as a value emitted
the source-level name rather than its mangled C symbol, so a Flow callback
could not be handed to a C dispatcher, and lib/scikit/threading.flow had
wrapped a GCD and pthreads parallel-for that consequently never had a
caller. Fixed upstream in Flow commit d62789dd, with a regression test and
all 705 tier-2 tests passing. The CI toolchain pin has to name a commit
containing that fix, which AGENTS.md now records along with the extra link
flag every build needs.

The forest is bit-identical. Every tree's bootstrap rows and feature seed
are drawn on one thread before any tree is fitted, in the same order the
sequential loop drew them, so the result does not depend on which tree
finishes first. Scratch moved from shared to per-task, since two trees
fitting at once would otherwise write the same buffers; it is one
allocation set per tree rather than per node. tests/test_opt_random
forestclassifier_fit compares against an independent reference forest field
by field and passes, the learned-state diagnostics on all 19 canonical rows
are unchanged, and repeated runs give the same forest.

The comparison is no longer like-for-like on this row, and the disparity
report now says so. Flow fits trees concurrently; scikit-learn's default is
one worker and the benchmark leaves it at its default, so both RandomForest
rows carry a declared n_jobs difference. Single-threaded the row is 1.82x,
so it wins either way. Asking scikit-learn for all cores does not close the
gap: at n_jobs=-1 its fit measured slower than at n_jobs=1, because
joblib's pool costs more than ten small trees save.

114/114 tests and examples pass. 11 process repeats against a prebuilt
binary, parity gate clean on all 19 rows, IQR at or under 8.5% on every
digits row.

* ci: pin the toolchain to a Flow build that can take a function's address

RandomForest fits its trees through flow_parallel_for, which hands a Flow
function to a C dispatcher. The pinned toolchain predates the compiler fix
that makes that compile, so CI would have failed on ensemble.flow with
"use of undeclared identifier".

The pin now names 5a0af023, the head of Flow PR #846. Move it to a commit
on Flow main once that merges. AGENTS.md records the requirement and the
follow-up, and its #843 entry is no longer describing an open bug.

Verified against that exact commit rather than against the local working
tree: 114/114 tests and examples pass with FLOW_BIN pointing at it, the
benchmark builds and runs, and the contract, disparity and publish jobs
pass locally step for step.

* ci: link the timing and threading shims in the KMeans parity workflow too

The canonical benchmark now calls flow_parallel_for and flow_now_ns, so
every job that builds bench_flow_v2.flow has to link both C shims.
remaining-issues.yml had its own FLOW_LDFLAGS that the earlier sweep
missed, and its build failed at the link step:

    undefined reference to `flow_parallel_for'
    undefined reference to `flow_now_ns'

All four FLOW_LDFLAGS declarations across the workflows now match.

Caught by CI rather than locally, because that workflow has no local
equivalent to run.

* test: record the RandomForest n_jobs difference in the config fixture

test_config_equivalence pins the configuration differences that survive on
the canonical rows, so that a new one has to be declared rather than
appearing silently. Both RandomForest rows now carry n_jobs, and the
fixture did not know about it.

Flow fits a forest's trees concurrently and scikit-learn's default is a
single worker, which the benchmark leaves at its default. That is a real
difference in what the two sides do, so it is reported rather than mapped
away as an equivalence, and the surviving-difference count goes from 4 to
6.

Caught by CI. It was failing locally too, and I had read the tail of its
output instead of its exit code.

* docs: record that the win count depends on the machine

The run committed here was measured on an Apple M4 Max with Accelerate and
wins all 19 rows. CI ran the same commit on a 4-core Intel Xeon with
OpenBLAS and won 18, losing LogisticRegression on digits at 0.94x, a row
that is 1.18x on the Mac.

Neither number is wrong. A row whose margin sits near 1x can land either
way on a different BLAS and core count, and the parity contract gates on
correctness and measurement resolution rather than on the win count, which
is why every check passed with the two runs disagreeing. CI freezes its own
measurement onto main after a merge, so the committed numbers there end up
being the runner's.

Better to say this in the README than to let a reader take one machine's
count as the property of the library.

* perf: stop the logistic fit on the criterion its tolerance names

LogisticRegression on digits was the row CI lost at 0.94x. It is now 3.18x,
and its score agrees with scikit-learn to 1.3e-08 rather than 2.8e-03,
which was the largest score gap in the suite.

Both came from the same cause. The multinomial fit tested the L2 norm of
the whole gradient against 1e-4. Over 650 parameters that norm runs about
an order of magnitude above the largest single component, so the fit was
held to a far tighter standard than its tolerance names, and it ran to its
100-iteration cap with the norm still falling. scipy's L-BFGS-B, which is
what scikit-learn's LogisticRegression runs, stops on the largest gradient
component at pgtol. Both objectives are the mean loss with
l2 = 1 / (C * n_samples), so the two gradients are on the same scale and
the same threshold means the same thing.

Stopping on the largest component takes the fit from 100 iterations to 31.
scikit-learn reports 31 on the same data.

The fit also took the raw LBFGS step of 1.0 with no line search, which the
parity contract recorded as an optimizer difference. It now backtracks on
the Armijo condition. An accepted step costs nothing extra, because the
objective evaluation leaves the probabilities where the gradient needs
them; only a rejected step repeats work. Both sides now record plain lbfgs
and that declared difference is gone, leaving max_iter as the only one on
those rows.

The objective uses logf rather than log on a widened argument, for the same
reason the softmax uses expf.

The one-vs-rest binary path had the same stopping rule and is aligned with
it, so the two paths stop on the same criterion.

114/114 tests and examples pass, including all nine logistic tests. Of the
19 canonical rows only this one moved, and it moved from 2.8e-03 to 1.3e-08
against scikit-learn. 11 process repeats, IQR at or under 8.8% on every
digits row.

* docs: record the four machines the headline has been measured on

After the convergence fix, LogisticRegression on digits wins on every
machine tried: 3.18x on an M4 Max with Accelerate, 2.87x on an Intel Xeon
Platinum 8370C, and 23x on two AMD EPYC 7763 runners.

The 23x is not worth quoting and the README says so. scikit-learn's own fit
of that row takes about 25 ms on the Intel runner and about 181 ms on the
AMD one, for the same code and data, so that figure measures an OpenBLAS
path that suits the machine badly rather than anything Flow does well.
Flow's own time on the two is 8.9 ms and 7.5 ms.

The verdict on that row did change with the machine before this fix, which
is why the paragraph exists at all: 1.18x on the Mac and 0.94x on CI.

* ci: move the toolchain pin onto Flow main

Flow PR #846 merged as 144fd168, so the pin no longer has to name a PR
branch. AGENTS.md drops the follow-up note along with it.

Verified against that exact commit rather than the local tree: 114/114
tests and examples pass with FLOW_BIN pointing at it, and the canonical
benchmark builds and emits all 19 rows.

* ci: repoint the toolchain pin after Flow rewrote its history

Flow's history was rewritten to strip Claude Code session URLs from commit
messages, which changed every SHA from 2026-08-05 onward. The pin named
144fd168, which no longer exists.

The same commit is now 88aac509. Verified it carries the fix, the test file
and an identical tree, and that the canonical benchmark builds against it
and emits all 19 rows.

---------
godofecht added a commit that referenced this pull request Sep 7, 2026
The KMeans parity job's artifact is what the freeze step promotes to
headline_result_v2.json, so it decides the published numbers. It was still
building Flow at -O0 against scikit-learn's optimized wheels, which is the
comparison the -O3 move corrected in flow.yml but not here.

The effect was visible the moment #504 merged. main froze 16 of 19 with
LinearSVC on digits at 510 ms and DecisionTree at 27.8 ms, against 155 ms
and 9.9 ms for the same commit in the optimized benchmark job, and against
19 of 19 on four other machines.

flow.yml:111 stays at -O0 on purpose. That job runs the test suite rather
than the benchmark, and compiles faster for it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant