Repository navigation
fix(ranking): prune zero-support rules in prune_harmful_rules - #26
AZERDSQ131 wants to merge 1 commit into
Conversation
Rules with validated_support == 0 never match anything on the dev split, so removing them changes ensemble F1 by exactly 0.0 -- which isn't < min_marginal_f1, so they survived as dead weight cluttering the rule report and rule count. Add a min_support parameter (default 1, conservative: a single dev match is enough to keep a rule) that also drops rules below that support threshold, independent of the existing marginal-F1 check. Closes #4
|
Reviewed — this is right, and it matches what #4 asked for: zero-support rules have marginal F1 of exactly 0.0 so the old threshold never caught them. Verified the only library call site (engine.rank_rules) stamps validated_support via rank_rules before pruning, so the new default is safe there. Tests and typecheck are green. One thing blocks the merge: the rights checkbox in the PR body (the failing required check). Please tick the '- [x] I certify...' box in the PR description (same for your other open PRs) and this can go in. |
|
One clarification since your PRs didn't start from the template — there's no box to tick, you need to add this exact line to the PR description (here and on #28/#29):
The rights-checkbox check re-runs when the description is edited, and #26/#28 merge as soon as it's green. |
|
Closing after three weeks without the rights attestation — we need that line from the author before we can take the code, so the same design has been reimplemented independently in #44 (crediting the stall, not the code). Thanks for the work regardless; with the attestation added, future PRs from you are very welcome. |
Problem
Rules with
validated_support == 0never match anything on the dev split, butprune_harmful_rulesonly drops a rule when its marginal F1 is known and strictly negative. A zero-support rule's marginal F1 is exactly0.0(removing it changes nothing), which is not< min_marginal_f1, so it survives as dead weight.Fix
Added a
min_support: int = 1parameter toprune_harmful_rules. Rules whosevalidated_supportis below this threshold are now dropped too, independent of the existing marginal-F1 logic (unchanged). Default of1is conservative — a single dev match is enough to keep a rule.Tests
test_prunes_zero_support_rule: a rule that never matches the dataset gets dropped with the defaultmin_support=1.test_min_support_zero_keeps_zero_support_rule:min_support=0disables the new check, preserving the old behavior.prune_harmful_rulestests still pass unchanged (their rules all have support ≥ 1).pytest tests/ -q→ 313 passed, 1 skipped.ruff checkandmypy rulechef/ranking.pyclean.Closes #4