fix(ranking): prune zero-support rules in prune_harmful_rules#26
Open
AZERDSQ131 wants to merge 1 commit into
Open
fix(ranking): prune zero-support rules in prune_harmful_rules#26AZERDSQ131 wants to merge 1 commit into
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 KRLabsOrg#4
Contributor
|
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. |
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.
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