-
Notifications
You must be signed in to change notification settings - Fork 4
feat: redundant rule detection via polynomial overhead comparison (#193) #194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
27 commits
Select commit
Hold shift + click to select a range
d3d974d
docs: add design for redundant rule detection utility (#193)
GiggleLiu a2b2724
docs: add implementation plan for redundant rule detection (#193)
GiggleLiu 5396e98
feat: add polynomial comparison for overhead analysis (#193)
GiggleLiu bc0458e
test: add integration test for dominated rule detection (#193)
GiggleLiu e569c8f
chore: clippy and format fixes (#193)
GiggleLiu a030bec
feat: remove redundant reduction rules (#193)
GiggleLiu 79397df
save
GiggleLiu 396193d
feat: refactor ILP to ILP<V> with VariableDomain trait
GiggleLiu f5ecd3c
refactor: update 9 reductions to target ILP<bool>
GiggleLiu 857e538
refactor: update Factoring → ILP<i32> with carry bounds as constraints
GiggleLiu 8be34e1
refactor: ILP<bool> → QUBO with compile-time binary guarantee
GiggleLiu efd52e4
feat: add ILP<bool> → ILP<i32> cast reduction
GiggleLiu 67e47f4
refactor: update ILP solver for generic ILP<V>
GiggleLiu f560ad4
test: update all tests for ILP<V> refactor
GiggleLiu 3f2c602
feat: remove ILP→QUBO from untrusted edges
GiggleLiu d324570
docs: update paper and generated artifacts for ILP<V>
GiggleLiu 2baa18e
chore: rustfmt formatting fixes
GiggleLiu dc0cb54
clean up rules
GiggleLiu 6cfed9a
Merge remote-tracking branch 'origin/main' into jg/redundant-rule-det…
GiggleLiu 0f4c2ce
update
GiggleLiu 624ea83
update
GiggleLiu 3e65229
update
GiggleLiu 4069b52
update
GiggleLiu 368559f
update
GiggleLiu 68d662a
update docs
GiggleLiu 417c122
fix: address PR #194 review comments
GiggleLiu e7b2609
fix: update CLI tests for changed reduction graph paths
GiggleLiu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| --- | ||
| name: check-rule-redundancy | ||
| description: Use when checking if a reduction rule (source-target pair) is redundant — i.e., dominated by a composite path through other rules in the reduction graph | ||
| --- | ||
|
|
||
| # Check Rule Redundancy | ||
|
|
||
| Determines whether reduction rules are redundant (dominated by composite paths through the reduction graph). Can check a single source-target pair or all primitive rules at once. | ||
|
|
||
| ## Invocation | ||
|
|
||
| ``` | ||
| /check-rule-redundancy # Check ALL primitive rules | ||
| /check-rule-redundancy <source> <target> # Check a specific rule | ||
| ``` | ||
|
|
||
| Examples: | ||
| ``` | ||
| /check-rule-redundancy | ||
| /check-rule-redundancy MIS ILP | ||
| /check-rule-redundancy MaximumIndependentSet QUBO | ||
| ``` | ||
|
|
||
| ## Mode 1: Check All Rules (no arguments) | ||
|
|
||
| When invoked without arguments, run the codebase's `find_dominated_rules` analysis test directly: | ||
|
|
||
| ```bash | ||
| cargo test test_find_dominated_rules_returns_known_set -- --nocapture 2>&1 | ||
| ``` | ||
|
|
||
| This runs the analysis from `src/rules/analysis.rs` which: | ||
| 1. Enumerates every primitive reduction rule (direct edge) in the graph | ||
| 2. For each, finds all alternative composite paths | ||
| 3. Uses polynomial normalization and monomial-dominance to compare overheads | ||
| 4. Reports dominated rules and unknown comparisons | ||
|
|
||
| Always report rules with full variant-qualified endpoints, not just base names. | ||
| Use the same display style as `ReductionStep`, e.g. | ||
| `MaximumIndependentSet {graph: "SimpleGraph", weight: "One"} -> MaximumIndependentSet {graph: "KingsSubgraph", weight: "i32"}`. | ||
| Base-name-only summaries are ambiguous and can hide cast-only paths. | ||
|
|
||
| Parse the test output and report a summary: | ||
|
|
||
| ```markdown | ||
| ## All Primitive Rules — Redundancy Report | ||
|
|
||
| ### Dominated Rules (N) | ||
|
|
||
| | # | Rule | Dominating Path | | ||
| |---|------|-----------------| | ||
| | 1 | Source {variant...} -> Target {variant...} | A -> B -> C | | ||
|
|
||
| ### Unknown Comparisons (N) | ||
|
|
||
| | # | Rule | Reason | | ||
| |---|------|--------| | ||
| | 1 | Source {variant...} -> Target {variant...} | expression comparison returned Unknown | | ||
|
|
||
| ### Allowed (acknowledged) dominated rules | ||
|
|
||
| List the entries from the `allowed` set in `test_find_dominated_rules_returns_known_set` | ||
| (file: `src/unit_tests/rules/analysis.rs`), and note when that allow-list is keyed only by base names while the reported dominated rule is variant-specific. | ||
|
|
||
| ### Verdict | ||
|
|
||
| - If test passes: all dominated rules are acknowledged in the allow-list. | ||
| - If test fails: report the unexpected dominated rule or stale allow-list entry. | ||
| ``` | ||
|
|
||
| ## Mode 2: Check Single Rule (source target arguments) | ||
|
|
||
| ### Step 1: Resolve Problem Names | ||
|
|
||
| Use MCP tools (`show_problem`) to validate and resolve aliases (MIS = MaximumIndependentSet, MVC = MinimumVertexCover, SAT = Satisfiability, etc.). | ||
|
|
||
| ### Step 2: Check if Rule Already Exists | ||
|
|
||
| Use `show_problem` on the source and check its `reduces_to` array for a direct edge to the target. | ||
|
|
||
| - **Direct edge exists**: Report "Direct rule `<source> -> <target>` already exists" and proceed to redundancy analysis (Step 3). | ||
| - **No direct edge**: Report "No direct rule from `<source> -> <target>` exists yet." Then check if any path exists: | ||
| - Use `find_path` MCP tool. | ||
| - **Path exists**: Report the cheapest existing path and its overhead. This is the baseline the proposed new rule must beat to be non-redundant. | ||
| - **No path exists**: Report "No path exists — a new rule would be novel (not redundant)." Stop here. | ||
|
|
||
| ### Step 3: Find All Paths | ||
|
|
||
| Use `find_path` with `all: true` to get all paths between source and target. | ||
|
|
||
| ### Step 4: Compare Overheads | ||
|
|
||
| For each composite path (length > 1 step): | ||
|
|
||
| 1. Extract the **overall overhead** from the path result | ||
| 2. Extract the **direct rule's overhead** from the single-step path | ||
| 3. Compare field by field: | ||
| - For polynomial expressions: compare degree — lower degree means the composite is better | ||
| - For equal-degree polynomials: compare leading coefficients | ||
| - For non-polynomial (exp, log): report as "Unknown — manual review needed" | ||
|
|
||
| **Dominance definition:** A composite path **dominates** the direct rule if, on every common overhead field, the composite's expression has equal or smaller asymptotic growth. | ||
|
|
||
| ### Step 5: Report Results | ||
|
|
||
| Output a structured report: | ||
|
|
||
| ```markdown | ||
| ## Redundancy Check: <Source> -> <Target> | ||
|
|
||
| ### Direct Rule | ||
| - Overhead: [field = expr, ...] | ||
| - Rule: `Source {variant...} -> Target {variant...}` | ||
| - Overhead: [field = expr, ...] | ||
|
|
||
| ### Composite Paths Found: N | ||
|
|
||
| | # | Path | Steps | Overhead | Comparison | | ||
| |---|------|-------|----------|------------| | ||
| | 1 | A -> B -> C | 2 | field = expr | Dominates / Worse / Unknown | | ||
|
|
||
| ### Verdict | ||
|
|
||
| - **Redundant**: At least one composite path dominates the direct rule | ||
| - **Not Redundant**: No composite path dominates the direct rule | ||
| - **Inconclusive**: Some paths have Unknown comparison (non-polynomial overhead) | ||
|
|
||
| ### Recommendation | ||
|
|
||
| If redundant: | ||
| > The direct rule `Source {variant...} -> Target {variant...}` is dominated by the composite path `[path]`. | ||
| > Consider removing it unless it provides value for: | ||
| > - Simpler solution extraction (fewer intermediate steps) | ||
| > - Educational/documentation clarity | ||
| > - Better numerical behavior in practice | ||
|
|
||
| If not redundant: | ||
| > The direct rule `Source {variant...} -> Target {variant...}` is not dominated by any composite path. | ||
| > It provides overhead that cannot be achieved through existing reductions. | ||
| ``` | ||
|
|
||
| ## Notes | ||
|
|
||
| - "Equal overhead" does not necessarily mean the rule should be removed — direct rules have practical advantages (simpler extraction, fewer steps) | ||
| - The analysis uses asymptotic comparison (big-O), so constant factors are ignored | ||
| - This means the check can produce false alarms, especially when overhead metadata keeps only leading terms or when a long composite path is asymptotically comparable but practically much worse | ||
| - Treat "dominated" as "potentially redundant, requires manual review" unless the composite path is also clearly preferable structurally | ||
| - When overhead expressions involve variables from different problems (e.g., `num_vertices` vs `num_clauses`), comparison may not be meaningful — report as Unknown | ||
| - The ground truth for what the codebase considers dominated is `src/rules/analysis.rs` (`find_dominated_rules`) with the allow-list in `src/unit_tests/rules/analysis.rs` (`test_find_dominated_rules_returns_known_set`) |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.