fix: compare benchmark configuration under declared equivalences (#469) - #474
Merged
Conversation
The disparity report compared each row's Flow and scikit-learn configuration records key by key, so it reported a difference every time the two projects spelled the same setting differently. 9 of 19 canonical rows carried a "configuration" dimension and most of the entries were name mismatches. The differences that are real were buried among them. benchmarks/config_equivalence.py holds the equivalences the comparator may apply. Three rules, each limited to a parameter present on exactly one side: * Cross-vocabulary mappings, verified numerically. sklearn's C and Flow's l2 name the same setting under alpha = 1 / (C * n_train), established in #430. They count as equivalent only when the recorded numbers satisfy the relation to 1e-5 relative, with n_train read from split_indices.json. A mapping that does not hold is still reported, with the value it demanded attached. * Absent equals explicitly disabled: LinearRegression's penalty=none against a sklearn OLS that has no penalty parameter. * Solver-private parameters: learning_rate against a counterpart solving with lbfgs, a direct factorization or coordinate descent; dual against a Flow LinearSVC with no primal/dual switch. max_iter is deliberately not in the solver-private table, and a parameter both sides record is always compared, so max_iter 200 vs 1000 and optimizer lbfgs_no_line_search vs lbfgs survive on both LogisticRegression rows and KernelSVC_RBF iris. Applied equivalences are written to each row's configuration_equivalences so the evidence is reclassified rather than dropped. Report schema_version 4. rows_with_configuration_difference 9 -> 5. Every other row field is byte identical. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #469
The pipeline, since the issue guessed at it
There are no
CONFIG|records. Neither runner emits one:bench_sklearn_v2.pyprints
RESULT|,DETAIL|,TIMING_UNIT|,BENCHMARK_MODE|,FIXTURE_SOURCE|andBENCHMARK_ENV|, andflow_results_v2.txtcontains onlythose prefixes. Configuration is declared, not measured:
config_diffwas a plain key-by-key dict comparison, so anything the twoprojects spelled differently read as a difference. Nothing else in the repo
constructs or consumes a configuration record, so the fix is contained to the
comparator.
bench_flow_v2.flowis untouched; noRESULT|orDETAIL|recordcan move, and compiler bug #469 does not apply to this change.
What changed
New
benchmarks/config_equivalence.pyholds the equivalences the comparator isallowed to apply. Three rules, in order, and each applies only to a parameter
present on exactly one side (or to a mapped pair each of whose names appears on
exactly its own side):
Cand Flow'sl2name the same setting underalpha = 1 / (C * n_train), the relationfix: match sklearn's C=1.0 in the canonical LogisticRegression benchmark (#408) #430 derived when closing LogisticRegression coefficient norm is 2x sklearn's at identical accuracy, suggesting the penalty is not being applied equivalently #408. They are equivalent only when the recorded
numbers satisfy it to
1e-5relative.n_trainis read fromsplit_indices.json(iris 120, digits 1437, diabetes 353), not hardcoded. Ifthe split fixture is missing or the row's dataset is not in it,
n_trainisunknown, the conversion cannot be verified and the pair stays reported.
penalty: noneon Flow'sLinearRegression against a sklearn OLS with no penalty parameter.
Falseisdeliberately excluded: a boolean set to false still implies the knob exists.
with why.
learning_rate(a step size only exists for a first-orderiterative solver; the sklearn counterparts here are line-searched lbfgs, a
direct factorization and coordinate descent) and
dual(sklearn'sprimal/dual liblinear switch; Flow implements one formulation). The rule is
gated on solver identity where the record carries it: if the counterpart
declares an
optimizer/solverthat does own the knob and still omits thevalue, the omission stays reported.
A parameter both sides record is always compared and any difference reported.
That is what keeps
max_iterandoptimizeralive.Applied equivalences are not dropped. Each row gains
configuration_equivalencescarrying the rule, the values and the reason, sothe report reclassifies the entry rather than erasing it. Report
schema_version3 -> 4.max_iteris deliberately not in the solver-private tableRidge/diabetes keeps a one-sided
max_iter 1000 vs <missing>after this change.scikit-learn's
Ridgedoes take amax_iter; the contract simply does notdeclare one, and an iteration budget is exactly the class of setting #469 asks
to keep visible. Flow solving Ridge in 1000 gradient steps where sklearn uses a
direct factorization is a real difference and should stay on the report.
Before / after, all 9 rows that carried a configuration dimension
Generated by running the
origin/maincopy ofgenerate_disparity_report.pyand the new one against the same committed inputs.
max_iter,optimizermax_iter,optimizermax_itersolvermax_iterFull entries:
Counts:
disparity_dimensionsdropsconfigurationon exactly four rows: LinearSVCiris, LinearSVC digits, Lasso diabetes, LinearRegression diabetes. No other
dimension moves on any row. Every row field outside
configuration_differences/configuration_equivalences/disparity_dimensionsis identical between the two reports, verified field byfield.
Proof the equivalence check bites
parity_contract.jsonwas temporarily patched so the iris LogisticRegressionrow records
l2: 0.01against sklearn'sC: 1.0, then the report wasregenerated:
Both entries come back, each carrying the value the relation demanded. The
contract was restored with
git checkout --andgit statusis clean of it;the committed
parity_contract.jsonis unchanged in this diff.benchmarks/test_config_equivalence.pypins the same property without touchingthe contract: the pre-#430 value (
l2=0.001againstC=1.0on iris, the 8.3xmismatch #408 was about) is reported, an unknown
n_trainis reported, and thedigits
n_trainapplied to the iris row is reported.Verification
Everything below was run locally on this branch.
The regression gate's configuration rule is one-sided (
new_n - old_n > max_increase), so counts going down cannot trip it. Row keys, score fields andruntime fields are untouched, and the compact history snapshot keeps the same
shape.
benchmarks/test_config_equivalence.pyis added to thesyntax-and-importsjob, next to the two existing benchmark fixture tests, and both new files are
added to that job's
py_compilelist.No generated artifact is in this diff.
disparity_report.json,parity_diagnostics.json,disparity_history.jsonandmodel_state_coverage.jsonwere all written to a scratch path outside therepository;
git statusis clean apart from the six files below.I have not run CI;
gh pr checkshas not been consulted at the time of writing.🤖 Generated with Claude Code