diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 867340d42..f5517cae1 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -15,6 +15,8 @@ Rust library for NP-hard problem reductions. Implements computational problems w - [check-issue](skills/check-issue/SKILL.md) -- Quality gate for `[Rule]` and `[Model]` issues. Checks usefulness, non-triviality, correctness of literature, and writing quality. Posts structured report and adds failure labels. - [check-rule-redundancy](skills/check-rule-redundancy/SKILL.md) -- Check if a reduction rule (source-target pair) is redundant, i.e., dominated by a composite path through other rules. - [meta-power](skills/meta-power/SKILL.md) -- Batch-resolve all open `[Model]` and `[Rule]` issues autonomously: plan, implement, review, fix CI, merge — in dependency order (models first). +- [project-pipeline](skills/project-pipeline/SKILL.md) -- Pick a Ready issue from the GitHub Project board, move it through In Progress -> issue-to-pr --execute -> review-agentic. +- [review-pipeline](skills/review-pipeline/SKILL.md) -- Pick a PR from review-agentic column, fix Copilot review comments, fix CI, run agentic feature tests, move to In Review. ## Commands ```bash @@ -42,6 +44,10 @@ make cli-demo # Run closed-loop CLI demo (exercises all commands) make mcp-test # Run MCP server tests (unit + integration) make run-plan # Execute a plan with Claude autorun make run-issue N=42 # Run issue-to-pr --execute for a GitHub issue +make run-pipeline # Pick next Ready issue from project board, implement, move to review-agentic +make run-pipeline N=97 # Process a specific issue from the project board +make run-review # Pick next PR from review-agentic column, fix Copilot comments, fix CI, run agentic tests +make run-review N=570 # Process a specific PR from the review-agentic column make copilot-review # Request Copilot code review on current PR make release V=x.y.z # Tag and push a new release (CI publishes to crates.io) ``` diff --git a/.claude/skills/issue-to-pr/SKILL.md b/.claude/skills/issue-to-pr/SKILL.md index 57f7cb27f..a03349cda 100644 --- a/.claude/skills/issue-to-pr/SKILL.md +++ b/.claude/skills/issue-to-pr/SKILL.md @@ -203,62 +203,13 @@ git push make copilot-review ``` -#### 7e. Fix Loop (max 3 retries) - -```bash -REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner) -``` - -For each retry: - -1. **Wait for CI to complete** (poll every 30s, up to 15 minutes): - ```bash - for i in $(seq 1 30); do - sleep 30 - HEAD_SHA=$(gh api repos/$REPO/pulls/$PR | python3 -c "import sys,json; print(json.load(sys.stdin)['head']['sha'])") - STATUS=$(gh api repos/$REPO/commits/$HEAD_SHA/check-runs | python3 -c " - import sys,json - runs = json.load(sys.stdin)['check_runs'] - if not runs: - print('PENDING') # CI hasn't registered yet - else: - failed = [r['name'] for r in runs if r.get('conclusion') not in ('success', 'skipped', None)] - pending = [r['name'] for r in runs if r.get('conclusion') is None and r['status'] != 'completed'] - if pending: - print('PENDING') - elif failed: - print('FAILED') - else: - print('GREEN') - ") - if [ "$STATUS" != "PENDING" ]; then break; fi - done - ``` - - - If `GREEN` on the **first** iteration (before any fix-pr): skip the fix loop, done. - - If `GREEN` after a fix-pr pass: break, done. - - If `FAILED`: continue to step 2. - - If still `PENDING` after 15 min: treat as `FAILED`. - -2. **Invoke `/fix-pr`** to address review comments, CI failures, and coverage gaps. - -3. **Push fixes:** - ```bash - git push - ``` - -4. Increment retry counter. If `< 3`, go back to step 1. If `= 3`, give up. - -**After 3 failed retries:** leave PR open, report to user. - -#### 7f. Done +#### 7e. Done Report final status: -- PR URL -- CI status (green / failed after retries) -- Any unresolved review items +- PR URL and number +- Implementation summary -The PR is **not merged** — the user or `meta-power` decides when to merge. +The PR is **not merged** and CI/review fixes are **not** handled here. The separate `review-pipeline` skill picks up PRs from the `review-agentic` board column to handle Copilot review comments, CI fixes, and agentic testing. ## Example @@ -286,9 +237,9 @@ Executing plan via subagent-driven-development... [Subagents implement the plan steps] [Runs review-implementation — all checks pass, auto-fixes applied] [Pushes + requests Copilot review] -[Polls CI... GREEN on first pass] -PR #45: CI green, ready for merge. +PR #45 created and pushed. Copilot review requested. +Run /review-pipeline to process Copilot comments, fix CI, and run agentic tests. ``` ## Common Mistakes diff --git a/.claude/skills/project-pipeline/SKILL.md b/.claude/skills/project-pipeline/SKILL.md new file mode 100644 index 000000000..9d29fbd2d --- /dev/null +++ b/.claude/skills/project-pipeline/SKILL.md @@ -0,0 +1,175 @@ +--- +name: project-pipeline +description: Pick a Ready issue from the GitHub Project board, move it through In Progress -> issue-to-pr -> review-agentic +--- + +# Project Pipeline + +Pick a "Ready" issue from the [GitHub Project board](https://github.com/orgs/CodingThrust/projects/8/views/1), move it to "In Progress", run `issue-to-pr --execute`, then move it to "review-agentic". The separate `review-pipeline` handles Copilot comments, CI fixes, and agentic testing. + +## Invocation + +- `/project-pipeline` -- pick the next Ready issue (first Model, then Rule, by issue number) +- `/project-pipeline 97` -- process a specific issue number from the Ready column +- `/project-pipeline --all` -- batch-process all Ready issues (Models first, then Rules) + +## Constants + +GitHub Project board IDs (for `gh project item-edit`): + +| Constant | Value | +|----------|-------| +| `PROJECT_ID` | `PVT_kwDOBrtarc4BRNVy` | +| `STATUS_FIELD_ID` | `PVTSSF_lADOBrtarc4BRNVyzg_GmQc` | +| `STATUS_READY` | `61e4505c` | +| `STATUS_IN_PROGRESS` | `47fc9ee4` | +| `STATUS_REVIEW_AGENTIC` | `b2f16561` | +| `STATUS_IN_REVIEW` | `df73e18b` | +| `STATUS_DONE` | `98236657` | + +## Autonomous Mode + +This skill runs **fully autonomously** — no confirmation prompts, no user questions. It picks the next issue and processes it end-to-end. All sub-skills (`issue-to-pr`, `check-issue`, `add-model`, `add-rule`, etc.) should also auto-approve any confirmation prompts. + +## Steps + +### 0. Discover Ready Issues + +```bash +gh project item-list 8 --owner CodingThrust --format json +``` + +Filter items where `status == "Ready"`. Partition into `[Model]` and `[Rule]` buckets, sort each by issue number ascending. Final order: **all Models first, then all Rules** (so dependencies are satisfied). + +Print the list for visibility (no confirmation needed): + +``` +Ready issues: + Models: + #129 [Model] MultivariateQuadratic + #117 [Model] GraphPartitioning + Rules: + #97 [Rule] BinPacking to ILP + #110 [Rule] LCS to ILP + #126 [Rule] KSatisfiability to SubsetSum + #130 [Rule] MultivariateQuadratic to ILP +``` + +**If a specific issue number was provided:** verify it is in the Ready column. If not, STOP with a message. + +**If `--all`:** proceed immediately with all Ready issues in order (no confirmation). + +**Otherwise (no args):** pick the first issue in the ordered list (Models before Rules, lowest number first) and proceed immediately (no confirmation). + +### 1. Create Worktree + +Create an isolated git worktree for this issue so the main working directory stays clean: + +```bash +git fetch origin main +BRANCH="issue--" +WORKTREE_DIR=".worktrees/$BRANCH" +mkdir -p .worktrees +git worktree add "$WORKTREE_DIR" -b "$BRANCH" origin/main +cd "$WORKTREE_DIR" +``` + +All subsequent steps run inside the worktree. This ensures the user's main checkout is never modified. + +### 2. Move to "In Progress" + +Extract the project item ID for the chosen issue from the JSON output (the `id` field of the matching item). + +```bash +gh project item-edit \ + --id \ + --project-id PVT_kwDOBrtarc4BRNVy \ + --field-id PVTSSF_lADOBrtarc4BRNVyzg_GmQc \ + --single-select-option-id 47fc9ee4 +``` + +### 3. Run issue-to-pr --execute + +Invoke the `issue-to-pr` skill with `--execute` (working directory is the worktree): + +``` +/issue-to-pr --execute +``` + +This handles the full pipeline: fetch issue, verify Good label, research, write plan, create PR, implement, review, fix CI. + +**If `issue-to-pr` fails:** record the failure, but still move the issue to "In Review" so it's visible for human triage. Report the failure to the user. + +### 4. Move to "review-agentic" + +After `issue-to-pr` completes (success or failure with a PR created), move the issue to the `review-agentic` column for the second-stage review pipeline: + +```bash +gh project item-edit \ + --id \ + --project-id PVT_kwDOBrtarc4BRNVy \ + --field-id PVTSSF_lADOBrtarc4BRNVyzg_GmQc \ + --single-select-option-id b2f16561 +``` + +**If no PR was created** (issue-to-pr failed before creating a PR): move the issue back to "Ready" instead: + +```bash +gh project item-edit \ + --id \ + --project-id PVT_kwDOBrtarc4BRNVy \ + --field-id PVTSSF_lADOBrtarc4BRNVyzg_GmQc \ + --single-select-option-id 61e4505c +``` + +### 5. Clean Up Worktree + +After the issue is processed (success or failure), clean up the worktree: + +```bash +cd /Users/liujinguo/rcode/problemreductions +git worktree remove "$WORKTREE_DIR" --force +``` + +### 6. Report (single issue) + +Print a summary: + +``` +Pipeline complete: + Issue: #97 [Rule] BinPacking to ILP + PR: #200 + Status: Awaiting agentic review + Board: Moved Ready -> In Progress -> review-agentic +``` + +### 7. Batch Mode (`--all`) + +If `--all` was specified, repeat Steps 1-6 for each issue in order. Each issue gets its own worktree (created and cleaned up per issue). + +After all issues, print a batch report: + +``` +=== Project Pipeline Batch Report === + +| Issue | Title | PR | Status | Board | +|-------|------------------------------------|------|-------------|-------------| +| #129 | [Model] MultivariateQuadratic | #201 | CI green | review-agentic | +| #97 | [Rule] BinPacking to ILP | #202 | CI green | review-agentic | +| #110 | [Rule] LCS to ILP | #203 | fix failed | review-agentic | +| #126 | [Rule] KSat to SubsetSum | - | plan failed | Ready | + +Completed: 2/4 | In Review: 3 | Returned to Ready: 1 +``` + +## Common Mistakes + +| Mistake | Fix | +|---------|-----| +| Issue not in Ready column | Verify status before processing; STOP if not Ready | +| Missing project scopes | Run `gh auth refresh -s read:project,project` | +| Forgetting to move back to Ready on total failure | Only move to In Review if a PR exists | +| Processing Rules before Models | Always sort Models first — Rules may depend on them | +| Not syncing main between batch issues | Each issue gets a fresh worktree from `origin/main` | +| Worktree left behind on failure | Always clean up with `git worktree remove` in Step 5 | +| Working in main checkout | All work happens in `.worktrees/` — never modify the main checkout | diff --git a/.claude/skills/review-pipeline/SKILL.md b/.claude/skills/review-pipeline/SKILL.md new file mode 100644 index 000000000..94b828bcf --- /dev/null +++ b/.claude/skills/review-pipeline/SKILL.md @@ -0,0 +1,198 @@ +--- +name: review-pipeline +description: Pick a PR from the review-agentic board column, fix Copilot review comments, fix CI, run agentic feature tests, then move to In Review +--- + +# Review Pipeline + +Pick PRs from the `review-agentic` column on the [GitHub Project board](https://github.com/orgs/CodingThrust/projects/8/views/1). For each PR: wait for Copilot review, fix comments, fix CI, run agentic feature tests, then move to `In Review`. + +## Invocation + +- `/review-pipeline` -- pick the next review-agentic item +- `/review-pipeline 570` -- process a specific PR number +- `/review-pipeline --all` -- batch-process all review-agentic items + +## Constants + +GitHub Project board IDs (for `gh project item-edit`): + +| Constant | Value | +|----------|-------| +| `PROJECT_ID` | `PVT_kwDOBrtarc4BRNVy` | +| `STATUS_FIELD_ID` | `PVTSSF_lADOBrtarc4BRNVyzg_GmQc` | +| `STATUS_REVIEW_AGENTIC` | `b2f16561` | +| `STATUS_IN_REVIEW` | `df73e18b` | +| `STATUS_READY` | `61e4505c` | + +## Autonomous Mode + +This skill runs **fully autonomously** -- no confirmation prompts, no user questions. + +## Steps + +### 0. Discover review-agentic Items + +```bash +gh project item-list 8 --owner CodingThrust --format json +``` + +Filter items where `status == "review-agentic"`. Each item should have an associated PR. Extract the PR number from the item title or linked issue. + +Print the list for visibility: + +``` +review-agentic PRs: + #570 Fix #117: [Model] GraphPartitioning + #571 Fix #97: [Rule] BinPacking to ILP +``` + +**If a specific PR number was provided:** verify it is in the review-agentic column. If not, STOP with a message. + +**If `--all`:** process all items in order (lowest PR number first). + +**Otherwise:** pick the first item. + +### 1. Create Worktree and Checkout PR Branch + +Create an isolated git worktree so the main working directory stays clean: + +```bash +REPO=$(gh repo view --json nameWithOwner --jq .nameWithOwner) +BRANCH=$(gh pr view $PR --json headRefName --jq .headRefName) +WORKTREE_DIR=".worktrees/review-$BRANCH" +mkdir -p .worktrees +git fetch origin $BRANCH +git worktree add "$WORKTREE_DIR" $BRANCH +cd "$WORKTREE_DIR" +``` + +All subsequent steps run inside the worktree. + +### 2. Fix Copilot Review Comments + +Check for existing Copilot review comments (no waiting — Copilot review was already requested by `issue-to-pr`): + +```bash +COMMENTS=$(gh api repos/$REPO/pulls/$PR/comments --jq '[.[] | select(.user.login == "copilot-pull-request-reviewer[bot]")]') +``` + +If there are actionable comments: invoke `/fix-pr` to address them, then push: + +```bash +git push +``` + +If no comments (or Copilot hasn't reviewed yet): skip to next step. + +### 3. Agentic Feature Test + +Run agentic feature tests on the modified feature: + +1. **Identify the feature** from the PR title and changed files: + - `[Model]` PRs: the new problem model name + - `[Rule]` PRs: the new reduction rule (source -> target) + +2. **Invoke `/agentic-tests:test-feature`** with the identified feature. This simulates a downstream user exercising the feature from docs and examples. + +3. **If test-feature reports issues:** fix them, commit, and push. + +4. **If test-feature passes:** continue to next step. + +### 4. Fix Loop (max 3 retries) + +For each retry: + +1. **Wait for CI to complete** (poll every 30s, up to 15 minutes): + ```bash + for i in $(seq 1 30); do + sleep 30 + HEAD_SHA=$(gh api repos/$REPO/pulls/$PR | python3 -c "import sys,json; print(json.load(sys.stdin)['head']['sha'])") + STATUS=$(gh api repos/$REPO/commits/$HEAD_SHA/check-runs | python3 -c " + import sys,json + runs = json.load(sys.stdin)['check_runs'] + if not runs: + print('PENDING') + else: + failed = [r['name'] for r in runs if r.get('conclusion') not in ('success', 'skipped', None)] + pending = [r['name'] for r in runs if r.get('conclusion') is None and r['status'] != 'completed'] + if pending: + print('PENDING') + elif failed: + print('FAILED') + else: + print('GREEN') + ") + if [ "$STATUS" != "PENDING" ]; then break; fi + done + ``` + + - If `GREEN` on the **first** iteration (before any fix-pr): skip the fix loop, done. + - If `GREEN` after a fix-pr pass: break, done. + - If `FAILED`: continue to step 2. + - If still `PENDING` after 15 min: treat as `FAILED`. + +2. **Invoke `/fix-pr`** to address CI failures and coverage gaps. + +3. **Push fixes:** + ```bash + git push + ``` + +4. Increment retry counter. If `< 3`, go back to step 1. If `= 3`, give up. + +**After 3 failed retries:** leave PR open, still move to In Review for human triage. + +### 5. Clean Up Worktree + +```bash +cd /Users/liujinguo/rcode/problemreductions +git worktree remove "$WORKTREE_DIR" --force +``` + +### 6. Move to "In Review" + +```bash +gh project item-edit \ + --id \ + --project-id PVT_kwDOBrtarc4BRNVy \ + --field-id PVTSSF_lADOBrtarc4BRNVyzg_GmQc \ + --single-select-option-id df73e18b +``` + +### 7. Report + +``` +Review pipeline complete: + PR: #570 + Copilot comments: 3 fixed + CI: green + Agentic test: passed + Board: review-agentic -> In Review +``` + +### 8. Batch Mode (`--all`) + +If `--all` was specified, repeat Steps 1-7 for each PR. After all PRs, print a batch report: + +``` +=== Review Pipeline Batch Report === + +| PR | Title | Copilot | CI | Agentic Test | Board | +|------|------------------------------------|---------|---------|--------------|------------| +| #570 | Fix #117: [Model] GraphPartitioning| 3 fixed | green | passed | In Review | +| #571 | Fix #97: [Rule] BinPacking to ILP | 0 | green | passed | In Review | + +Completed: 2/2 | All moved to In Review +``` + +## Common Mistakes + +| Mistake | Fix | +|---------|-----| +| PR not in review-agentic column | Verify status before processing; STOP if not review-agentic | +| Missing project scopes | Run `gh auth refresh -s read:project,project` | +| Skipping agentic tests | Always run test-feature even if CI is green | +| Not checking out the right branch | Use `gh pr view` to get the exact branch name | +| Worktree left behind on failure | Always clean up with `git worktree remove` in Step 5 | +| Working in main checkout | All work happens in `.worktrees/` — never modify the main checkout | diff --git a/Makefile b/Makefile index 2e9948c28..ec1708a89 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ # Makefile for problemreductions -.PHONY: help build test mcp-test fmt clippy doc mdbook paper examples clean coverage rust-export compare qubo-testdata export-schemas release run-plan run-issue diagrams jl-testdata cli cli-demo copilot-review +.PHONY: help build test mcp-test fmt clippy doc mdbook paper examples clean coverage rust-export compare qubo-testdata export-schemas release run-plan run-issue run-pipeline run-review diagrams jl-testdata cli cli-demo copilot-review # Default target help: @@ -29,6 +29,8 @@ help: @echo " cli-demo - Run closed-loop CLI demo (build + exercise all commands)" @echo " run-plan - Execute a plan with Claude autorun (latest plan in docs/plans/)" @echo " run-issue N= - Run issue-to-pr --execute for a GitHub issue" + @echo " run-pipeline [N=] - Pick a Ready issue, implement, move to review-agentic" + @echo " run-review [N=] - Pick PR from review-agentic, fix comments/CI, run agentic tests" @echo " copilot-review - Request Copilot code review on current PR" # Build the project @@ -361,6 +363,35 @@ cli-demo: cli echo "=== Demo complete: $$(ls $(CLI_DEMO_DIR)/*.json | wc -l | tr -d ' ') JSON files in $(CLI_DEMO_DIR) ===" @echo "=== All 20 steps passed ✅ ===" +# Run project-pipeline: pick a Ready issue, implement, move to In Review +# Usage: make run-pipeline (picks next Ready issue automatically) +# make run-pipeline N=97 (processes specific issue) +run-pipeline: + @if [ -n "$(N)" ]; then \ + PROMPT="/project-pipeline $(N)"; \ + else \ + PROMPT="/project-pipeline"; \ + fi; \ + claude --dangerously-skip-permissions \ + --model opus \ + --verbose \ + --max-turns 500 \ + -p "$$PROMPT" 2>&1 | tee "pipeline-output.log" + +# Usage: make run-review (picks next review-agentic PR automatically) +# make run-review N=570 (processes specific PR) +run-review: + @if [ -n "$(N)" ]; then \ + PROMPT="/review-pipeline $(N)"; \ + else \ + PROMPT="/review-pipeline"; \ + fi; \ + claude --dangerously-skip-permissions \ + --model opus \ + --verbose \ + --max-turns 500 \ + -p "$$PROMPT" 2>&1 | tee "review-output.log" + # Request Copilot code review on the current PR # Requires: gh extension install ChrisCarini/gh-copilot-review copilot-review: diff --git a/problemreductions-cli/src/bin/pred_sym.rs b/problemreductions-cli/src/bin/pred_sym.rs index 4f2d43a27..daa28bf7a 100644 --- a/problemreductions-cli/src/bin/pred_sym.rs +++ b/problemreductions-cli/src/bin/pred_sym.rs @@ -4,6 +4,7 @@ use problemreductions::{big_o_normal_form, canonical_form, Expr, ProblemSize}; #[derive(Parser)] #[command( name = "pred-sym", + version, about = "Symbolic expression engine for problemreductions" )] struct Cli { @@ -28,15 +29,19 @@ enum Commands { /// Expression string #[arg(name = "expr")] expr: String, + /// Output without O(...) wrapper + #[arg(long)] + raw: bool, }, - /// Compare two expressions + /// Compare two expressions (exits with code 1 if neither exact nor Big-O equal) Compare { /// First expression a: String, /// Second expression b: String, }, - /// Evaluate an expression with variable bindings + /// Evaluate an expression with variable bindings. + /// Supported functions: exp (e^x), log (natural log, base e), sqrt Eval { /// Expression string expr: String, @@ -46,16 +51,26 @@ enum Commands { }, } +fn parse_expr_or_exit(expr: &str) -> Expr { + match Expr::try_parse(expr) { + Ok(parsed) => parsed, + Err(e) => { + eprintln!("Error: failed to parse expression \"{expr}\": {e}"); + std::process::exit(2); + } + } +} + fn main() { let cli = Cli::parse(); match cli.command { Commands::Parse { expr } => { - let parsed = Expr::parse(&expr); + let parsed = parse_expr_or_exit(&expr); println!("{parsed}"); } Commands::Canon { expr } => { - let parsed = Expr::parse(&expr); + let parsed = parse_expr_or_exit(&expr); match canonical_form(&parsed) { Ok(result) => println!("{result}"), Err(e) => { @@ -64,10 +79,16 @@ fn main() { } } } - Commands::BigO { expr } => { - let parsed = Expr::parse(&expr); + Commands::BigO { expr, raw } => { + let parsed = parse_expr_or_exit(&expr); match big_o_normal_form(&parsed) { - Ok(result) => println!("O({result})"), + Ok(result) => { + if raw { + println!("{result}"); + } else { + println!("O({result})"); + } + } Err(e) => { eprintln!("Error: {e}"); std::process::exit(1); @@ -75,8 +96,8 @@ fn main() { } } Commands::Compare { a, b } => { - let expr_a = Expr::parse(&a); - let expr_b = Expr::parse(&b); + let expr_a = parse_expr_or_exit(&a); + let expr_b = parse_expr_or_exit(&b); let canon_a = canonical_form(&expr_a); let canon_b = canonical_form(&expr_b); let big_o_a = big_o_normal_form(&expr_a); @@ -84,19 +105,26 @@ fn main() { println!("Expression A: {a}"); println!("Expression B: {b}"); + let mut exact_equal = false; + let mut big_o_equal = false; if let (Ok(ca), Ok(cb)) = (&canon_a, &canon_b) { + exact_equal = ca == cb; println!("Canonical A: {ca}"); println!("Canonical B: {cb}"); - println!("Exact equal: {}", ca == cb); + println!("Exact equal: {exact_equal}"); } if let (Ok(ba), Ok(bb)) = (&big_o_a, &big_o_b) { + big_o_equal = ba == bb; println!("Big-O A: O({ba})"); println!("Big-O B: O({bb})"); - println!("Big-O equal: {}", ba == bb); + println!("Big-O equal: {big_o_equal}"); + } + if !exact_equal && !big_o_equal { + std::process::exit(1); } } Commands::Eval { expr, vars } => { - let parsed = Expr::parse(&expr); + let parsed = parse_expr_or_exit(&expr); let bindings: Vec<(&str, usize)> = vars .split(',') .filter_map(|pair| { @@ -108,6 +136,26 @@ fn main() { Some((leaked, value)) }) .collect(); + + // Check for unbound variables + let expr_vars = parsed.variables(); + let bound_vars: std::collections::HashSet<&str> = + bindings.iter().map(|(k, _)| *k).collect(); + let mut unbound: Vec<&str> = expr_vars + .iter() + .filter(|v| !bound_vars.contains(*v)) + .copied() + .collect(); + if !unbound.is_empty() { + unbound.sort(); + eprintln!( + "Error: unbound variable{}: {}", + if unbound.len() > 1 { "s" } else { "" }, + unbound.join(", ") + ); + std::process::exit(1); + } + let size = ProblemSize::new(bindings); let result = parsed.eval(&size); diff --git a/problemreductions-cli/tests/pred_sym_tests.rs b/problemreductions-cli/tests/pred_sym_tests.rs index 2c4103a83..64b424d2a 100644 --- a/problemreductions-cli/tests/pred_sym_tests.rs +++ b/problemreductions-cli/tests/pred_sym_tests.rs @@ -77,3 +77,119 @@ fn test_pred_sym_compare() { stdout.trim() ); } + +#[test] +fn test_pred_sym_parse_invalid_input_returns_error() { + let output = pred_sym().args(["parse", "n +"]).output().unwrap(); + assert!(!output.status.success()); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!( + stderr.contains("failed to parse expression"), + "got: {stderr}" + ); +} + +#[test] +fn test_pred_sym_big_o_rejects_division() { + let output = pred_sym().args(["big-o", "n / m"]).output().unwrap(); + assert!(!output.status.success()); +} + +#[test] +fn test_pred_sym_version() { + let output = pred_sym().args(["--version"]).output().unwrap(); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!(stdout.contains("pred-sym"), "got: {stdout}"); +} + +#[test] +fn test_pred_sym_big_o_raw() { + let output = pred_sym() + .args(["big-o", "--raw", "3 * n^2 + n"]) + .output() + .unwrap(); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert_eq!(stdout.trim(), "n^2"); +} + +#[test] +fn test_pred_sym_big_o_exp_dominates_poly() { + let output = pred_sym().args(["big-o", "2^n + n^3"]).output().unwrap(); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!(stdout.contains("2^n"), "got: {}", stdout.trim()); + assert!( + !stdout.contains("n^3"), + "n^3 should be dominated, got: {}", + stdout.trim() + ); +} + +#[test] +fn test_pred_sym_big_o_larger_base_dominates() { + let output = pred_sym().args(["big-o", "3^n + 2^n"]).output().unwrap(); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!(stdout.contains("3^n"), "got: {}", stdout.trim()); + assert!( + !stdout.contains("2^n"), + "2^n should be dominated, got: {}", + stdout.trim() + ); +} + +#[test] +fn test_pred_sym_big_o_poly_log_dominates_poly() { + let output = pred_sym() + .args(["big-o", "n * log(n) + n"]) + .output() + .unwrap(); + assert!(output.status.success()); + let stdout = String::from_utf8(output.stdout).unwrap(); + assert!( + stdout.contains("log"), + "expected n*log(n) to survive, got: {}", + stdout.trim() + ); +} + +#[test] +fn test_pred_sym_eval_unbound_variable_error() { + let output = pred_sym() + .args(["eval", "n^2", "--vars", "m=10"]) + .output() + .unwrap(); + assert!(!output.status.success(), "should fail for unbound variable"); + let stderr = String::from_utf8(output.stderr).unwrap(); + assert!( + stderr.contains("unbound variable"), + "expected unbound variable error, got: {stderr}" + ); + assert!( + stderr.contains("n"), + "should mention variable 'n', got: {stderr}" + ); +} + +#[test] +fn test_pred_sym_compare_unequal_exits_nonzero() { + let output = pred_sym().args(["compare", "n^2", "n^3"]).output().unwrap(); + assert!( + !output.status.success(), + "compare of unequal expressions should exit non-zero" + ); +} + +#[test] +fn test_pred_sym_compare_big_o_equal_exits_zero() { + let output = pred_sym() + .args(["compare", "2*n^2 + n", "n^2"]) + .output() + .unwrap(); + assert!( + output.status.success(), + "Big-O equal expressions should exit 0" + ); +} diff --git a/src/big_o.rs b/src/big_o.rs index 6068b1ad8..fa568074a 100644 --- a/src/big_o.rs +++ b/src/big_o.rs @@ -6,6 +6,12 @@ use crate::canonical::canonical_form; use crate::expr::{AsymptoticAnalysisError, CanonicalizationError, Expr}; +#[derive(Clone, Debug)] +struct ProjectedTerm { + expr: Expr, + negative: bool, +} + /// Compute the Big-O normal form of an expression. /// /// This is a two-phase pipeline: @@ -28,9 +34,9 @@ fn project_big_o(expr: &Expr) -> Result { collect_additive_terms(expr, &mut terms); // Project each term: drop constant multiplicative factors - let mut projected: Vec = Vec::new(); + let mut projected: Vec = Vec::new(); for term in &terms { - if let Some(projected_term) = project_term(term) { + if let Some(projected_term) = project_term(term)? { projected.push(projected_term); } // Pure constants are dropped (asymptotically irrelevant) @@ -44,20 +50,27 @@ fn project_big_o(expr: &Expr) -> Result { return Ok(Expr::Const(1.0)); } + if let Some(negative) = survivors.iter().find(|term| term.negative) { + return Err(AsymptoticAnalysisError::Unsupported(format!( + "-1 * {}", + negative.expr + ))); + } + // Deduplicate let mut seen = std::collections::BTreeSet::new(); let mut deduped = Vec::new(); for term in survivors { - let key = term.to_string(); + let key = term.expr.to_string(); if seen.insert(key) { deduped.push(term); } } // Rebuild sum - let mut result = deduped[0].clone(); + let mut result = deduped[0].expr.clone(); for term in &deduped[1..] { - result = result + term.clone(); + result = result + term.expr.clone(); } Ok(result) @@ -75,30 +88,41 @@ fn collect_additive_terms(expr: &Expr, out: &mut Vec) { /// Project a single multiplicative term: strip constant factors. /// Returns None if the term is a pure constant. -fn project_term(term: &Expr) -> Option { +fn project_term(term: &Expr) -> Result, AsymptoticAnalysisError> { if term.constant_value().is_some() { - return None; // Pure constant → dropped + return Ok(None); // Pure constant → dropped } // Collect multiplicative factors let mut factors = Vec::new(); collect_multiplicative_factors(term, &mut factors); - // Remove constant factors, keep symbolic ones - let symbolic: Vec<&Expr> = factors - .iter() - .filter(|f| f.constant_value().is_none()) - .collect(); + let mut coeff = 1.0; + let mut symbolic = Vec::new(); + for factor in &factors { + if let Some(c) = factor.constant_value() { + coeff *= c; + continue; + } + if contains_negative_exponent(factor) { + return Err(AsymptoticAnalysisError::Unsupported(term.to_string())); + } + symbolic.push(factor.clone()); + } if symbolic.is_empty() { - return None; + return Ok(None); } let mut result = symbolic[0].clone(); for f in &symbolic[1..] { - result = result * (*f).clone(); + result = result * f.clone(); } - Some(result) + + Ok(Some(ProjectedTerm { + expr: result, + negative: coeff < 0.0, + })) } fn collect_multiplicative_factors(expr: &Expr, out: &mut Vec) { @@ -115,7 +139,7 @@ fn collect_multiplicative_factors(expr: &Expr, out: &mut Vec) { /// /// A term `t` is dominated if there exists another term `s` such that /// `t` grows no faster than `s` asymptotically. -fn remove_dominated_terms(terms: Vec) -> Vec { +fn remove_dominated_terms(terms: Vec) -> Vec { if terms.len() <= 1 { return terms; } @@ -125,7 +149,7 @@ fn remove_dominated_terms(terms: Vec) -> Vec { let is_dominated = terms .iter() .enumerate() - .any(|(j, other)| i != j && term_dominated_by(term, other)); + .any(|(j, other)| i != j && term_dominated_by(&term.expr, &other.expr)); if !is_dominated { survivors.push(term.clone()); } @@ -135,24 +159,61 @@ fn remove_dominated_terms(terms: Vec) -> Vec { /// Check if `small` is asymptotically dominated by `big`. /// -/// Conservative: only returns true when dominance is provable -/// via monomial exponent comparison. +/// Supports three comparison strategies: +/// 1. Polynomial monomial exponent comparison (exact) +/// 2. Exponential vs subexponential / base comparison (structural) +/// 3. Numerical evaluation at two scales (for subexponential cross-class) fn term_dominated_by(small: &Expr, big: &Expr) -> bool { - // Extract monomial exponents for comparison + // Case 1: Both pure polynomial monomials — use exponent comparison let small_exps = extract_var_exponents(small); let big_exps = extract_var_exponents(big); + if let (Some(ref se), Some(ref be)) = (small_exps, big_exps) { + return polynomial_dominated(se, be); + } + + // Cross-class comparison: small's variables must be a subset of big's + let small_vars = small.variables(); + let big_vars = big.variables(); + if small_vars.is_empty() || big_vars.is_empty() || !small_vars.is_subset(&big_vars) { + return false; + } + + // Case 2: Exponential comparison + let small_has_exp = has_exponential_growth(small); + let big_has_exp = has_exponential_growth(big); + match (small_has_exp, big_has_exp) { + (false, true) => return true, // exponential dominates subexponential + (true, false) => return false, // subexponential can't dominate exponential + (true, true) => { + // Compare effective exponential bases + if let (Some(sb), Some(bb)) = (effective_exp_base(small), effective_exp_base(big)) { + if bb > sb * (1.0 + 1e-10) { + return true; + } + } + return false; + } + (false, false) => {} // both subexponential, fall through + } + + // Case 3: Both subexponential, same variables — numerical comparison + // Handles: poly vs poly*log, log vs log(log), poly vs log, etc. + if small_vars == big_vars { + return numerical_dominance_check(small, big, &small_vars); + } - // Both must be pure polynomial monomials for comparison - let (Some(se), Some(be)) = (small_exps, big_exps) else { - return false; // Can't compare non-polynomial terms - }; + false +} - // small ≤ big if: for every variable in small, big has ≥ exponent - // AND big has at least one strictly greater exponent or has a variable small doesn't +/// Check polynomial dominance: small ≤ big component-wise with at least one strict inequality. +fn polynomial_dominated( + se: &std::collections::BTreeMap<&'static str, f64>, + be: &std::collections::BTreeMap<&'static str, f64>, +) -> bool { let mut all_leq = true; let mut any_strictly_less = false; - for (var, small_exp) in &se { + for (var, small_exp) in se { let big_exp = be.get(var).copied().unwrap_or(0.0); if *small_exp > big_exp + 1e-15 { all_leq = false; @@ -163,17 +224,14 @@ fn term_dominated_by(small: &Expr, big: &Expr) -> bool { } } - // Also check variables in big not in small (those have implicit exponent 0 in small) if all_leq { - for (var, big_exp) in &be { + for (var, big_exp) in be { if !se.contains_key(var) && *big_exp > 1e-15 { any_strictly_less = true; } } } - // Dominated if all exponents ≤ AND at least one is strictly less. - // Equal terms are NOT dominated — they get deduped in a separate step. all_leq && any_strictly_less } @@ -197,6 +255,9 @@ fn extract_var_exponents_inner( } Expr::Pow(base, exp) => { if let (Expr::Var(name), Some(e)) = (base.as_ref(), exp.constant_value()) { + if e < 0.0 { + return None; + } *exps.entry(name).or_insert(0.0) += e; Some(()) } else { @@ -212,6 +273,115 @@ fn extract_var_exponents_inner( } } +fn contains_negative_exponent(expr: &Expr) -> bool { + match expr { + Expr::Pow(_, exp) => exp.constant_value().is_some_and(|e| e < 0.0), + Expr::Mul(a, b) | Expr::Add(a, b) => { + contains_negative_exponent(a) || contains_negative_exponent(b) + } + Expr::Exp(arg) | Expr::Log(arg) | Expr::Sqrt(arg) => contains_negative_exponent(arg), + Expr::Const(_) | Expr::Var(_) => false, + } +} + +/// Check if an expression has exponential growth. +/// +/// Returns true if the expression contains `exp(var_expr)` or `c^(var_expr)` where c > 1. +fn has_exponential_growth(expr: &Expr) -> bool { + match expr { + Expr::Exp(arg) => !arg.variables().is_empty(), + Expr::Pow(base, exp) => { + base.constant_value().is_some_and(|c| c > 1.0) && !exp.variables().is_empty() + } + Expr::Mul(a, b) => has_exponential_growth(a) || has_exponential_growth(b), + _ => false, + } +} + +/// Compute the effective exponential base for growth rate comparison. +/// +/// For `c^(f(n))`, approximates the effective base as `c^(f(1))`. +/// This works correctly for linear exponents (the common case in complexity expressions). +fn effective_exp_base(expr: &Expr) -> Option { + match expr { + Expr::Exp(arg) => { + let vars = arg.variables(); + if vars.is_empty() { + None + } else { + let size = unit_problem_size(&vars); + let rate = arg.eval(&size); + Some(std::f64::consts::E.powf(rate)) + } + } + Expr::Pow(base, exp) => { + if let Some(c) = base.constant_value() { + let vars = exp.variables(); + if c > 1.0 && !vars.is_empty() { + let size = unit_problem_size(&vars); + let exp_at_1 = exp.eval(&size); + Some(c.powf(exp_at_1)) + } else { + None + } + } else { + None + } + } + Expr::Mul(a, b) => match (effective_exp_base(a), effective_exp_base(b)) { + (Some(ba), Some(bb)) => Some(ba * bb), + (Some(b), None) | (None, Some(b)) => Some(b), + (None, None) => None, + }, + _ => None, + } +} + +/// Create a `ProblemSize` with all variables set to the given value. +fn make_problem_size( + vars: &std::collections::HashSet<&'static str>, + val: usize, +) -> crate::types::ProblemSize { + crate::types::ProblemSize::new(vars.iter().map(|&v| (v, val)).collect()) +} + +/// Create a `ProblemSize` with all variables set to 1. +fn unit_problem_size(vars: &std::collections::HashSet<&'static str>) -> crate::types::ProblemSize { + make_problem_size(vars, 1) +} + +/// Check dominance numerically by evaluating at two scales. +/// +/// Returns true if `big/small` ratio is > 1 and increasing between the two +/// evaluation points, indicating `big` grows asymptotically faster. +fn numerical_dominance_check( + small: &Expr, + big: &Expr, + vars: &std::collections::HashSet<&'static str>, +) -> bool { + let size1 = make_problem_size(vars, 100); + let size2 = make_problem_size(vars, 10_000); + + let s1 = small.eval(&size1); + let b1 = big.eval(&size1); + let s2 = small.eval(&size2); + let b2 = big.eval(&size2); + + // Both must be finite and positive at both points + if !s1.is_finite() || !b1.is_finite() || !s2.is_finite() || !b2.is_finite() { + return false; + } + if s1 <= 1e-300 || b1 <= 1e-300 || s2 <= 1e-300 || b2 <= 1e-300 { + return false; + } + + let ratio1 = b1 / s1; + let ratio2 = b2 / s2; + + // Dominance: ratio is > 1 at both points and strictly increasing + ratio1 > 1.0 + 1e-10 && ratio2 > ratio1 * (1.0 + 1e-6) +} + #[cfg(test)] #[path = "unit_tests/big_o.rs"] mod tests; diff --git a/src/canonical.rs b/src/canonical.rs index cc93d4536..ae96ac09f 100644 --- a/src/canonical.rs +++ b/src/canonical.rs @@ -32,6 +32,14 @@ impl Ord for OpaqueFactor { } } +fn normalized_f64_bits(value: f64) -> u64 { + if value == 0.0 { + 0.0f64.to_bits() + } else { + value.to_bits() + } +} + /// A single additive term: coefficient × product of canonical factors. #[derive(Clone, Debug)] struct CanonicalTerm { @@ -60,11 +68,11 @@ fn try_merge_opaque(existing: &[OpaqueFactor], new: &OpaqueFactor) -> Option c^(a+b) for matching constant base c + // c^a * c^b -> c^(a+b) for matching positive constant base c if let (Expr::Pow(base1, exp1), Expr::Pow(base2, exp2)) = (&existing_factor.expr, &new.expr) { if let (Some(c1), Some(c2)) = (base1.constant_value(), base2.constant_value()) { - if (c1 - c2).abs() < 1e-15 { + if c1 > 0.0 && c2 > 0.0 && (c1 - c2).abs() < 1e-15 { let merged_exp = (**exp1).clone() + (**exp2).clone(); let canon_exp = canonical_form(&merged_exp).unwrap_or(merged_exp); let merged_expr = Expr::Pow(base1.clone(), Box::new(canon_exp)); @@ -145,11 +153,11 @@ impl CanonicalTerm { } /// Deterministic sort key for ordering terms in a sum. - fn sort_key(&self) -> (Vec<(&'static str, i64)>, Vec) { + fn sort_key(&self) -> (Vec<(&'static str, u64)>, Vec) { let vars: Vec<_> = self .vars .iter() - .map(|(&k, &v)| (k, (v * 1000.0).round() as i64)) + .map(|(&k, &v)| (k, normalized_f64_bits(v))) .collect(); let opaque: Vec<_> = self.opaque.iter().map(|o| o.key.clone()).collect(); (vars, opaque) @@ -179,7 +187,7 @@ impl CanonicalSum { /// Merge terms with the same signature and drop zero-coefficient terms. /// Sort the result deterministically. fn simplify(self) -> Self { - type SortKey = (Vec<(&'static str, i64)>, Vec); + type SortKey = (Vec<(&'static str, u64)>, Vec); let mut groups: BTreeMap = BTreeMap::new(); for term in self.terms { @@ -299,6 +307,16 @@ fn canonicalize_pow(base: &Expr, exp: &Expr) -> Result { + let c = base.constant_value().unwrap(); + if (c - 1.0).abs() < 1e-15 { + return Ok(CanonicalSum::from_term(CanonicalTerm::constant(1.0))); + } + if c <= 0.0 { + return Err(CanonicalizationError::Unsupported(format!( + "{}^{}", + base, exp + ))); + } let canon_exp = canonical_form(exp)?; Ok(CanonicalSum::from_term(CanonicalTerm::opaque_factor( Expr::Pow(Box::new(base.clone()), Box::new(canon_exp)), diff --git a/src/expr.rs b/src/expr.rs index 7226d7102..0754dacf8 100644 --- a/src/expr.rs +++ b/src/expr.rs @@ -104,10 +104,15 @@ impl Expr { /// # Panics /// Panics if the expression string has invalid syntax. pub fn parse(input: &str) -> Expr { - parse_to_expr(input) + Self::try_parse(input) .unwrap_or_else(|e| panic!("failed to parse expression \"{input}\": {e}")) } + /// Parse an expression string into an `Expr`, returning a normal error on failure. + pub fn try_parse(input: &str) -> Result { + parse_to_expr(input) + } + /// Check if this expression is a polynomial (no exp/log/sqrt, integer exponents only). pub fn is_polynomial(&self) -> bool { match self { @@ -465,10 +470,10 @@ impl ExprParser { } fn parse_multiplicative(&mut self) -> Result { - let mut left = self.parse_power()?; + let mut left = self.parse_unary()?; while matches!(self.peek(), Some(ExprToken::Star) | Some(ExprToken::Slash)) { let op = self.advance().unwrap(); - let right = self.parse_power()?; + let right = self.parse_unary()?; left = match op { ExprToken::Star => left * right, ExprToken::Slash => left / right, @@ -479,10 +484,10 @@ impl ExprParser { } fn parse_power(&mut self) -> Result { - let base = self.parse_unary()?; + let base = self.parse_primary()?; if matches!(self.peek(), Some(ExprToken::Caret)) { self.advance(); - let exp = self.parse_power()?; // right-associative + let exp = self.parse_unary()?; // right-associative, allows unary minus in exponent Ok(Expr::pow(base, exp)) } else { Ok(base) @@ -495,7 +500,7 @@ impl ExprParser { let expr = self.parse_unary()?; Ok(-expr) } else { - self.parse_primary() + self.parse_power() } } diff --git a/src/unit_tests/big_o.rs b/src/unit_tests/big_o.rs index 1435771f4..85aefa139 100644 --- a/src/unit_tests/big_o.rs +++ b/src/unit_tests/big_o.rs @@ -84,14 +84,14 @@ fn test_big_o_composed_overhead_duplicate() { #[test] fn test_big_o_exp_with_polynomial() { - // exp(n) + n^10 — incomparable, both survive + // exp(n) dominates n^10 let e = Expr::Exp(Box::new(Expr::Var("n"))) + Expr::pow(Expr::Var("n"), Expr::Const(10.0)); let result = big_o_normal_form(&e).unwrap(); let s = result.to_string(); assert!(s.contains("exp"), "expected exp term to survive, got: {s}"); assert!( - s.contains("n"), - "expected polynomial term to survive, got: {s}" + !s.contains("n^10"), + "n^10 should be dominated by exp(n), got: {s}" ); } @@ -101,3 +101,119 @@ fn test_big_o_pure_constant_returns_one() { let result = big_o_normal_form(&e).unwrap(); assert_eq!(result.to_string(), "1"); } + +#[test] +fn test_big_o_rejects_division() { + let e = Expr::Var("n") / Expr::Var("m"); + assert!(big_o_normal_form(&e).is_err()); +} + +#[test] +fn test_big_o_rejects_negative_dominant_term() { + let e = Expr::Const(-1.0) * Expr::Var("n"); + assert!(big_o_normal_form(&e).is_err()); +} + +#[test] +fn test_big_o_constant_base_one_becomes_constant() { + let e = Expr::pow(Expr::Const(1.0), Expr::Var("n")); + let result = big_o_normal_form(&e).unwrap(); + assert_eq!(result.to_string(), "1"); +} + +#[test] +fn test_big_o_rejects_nonpositive_constant_base_exponential() { + let e = Expr::pow(Expr::Const(-2.0), Expr::Var("n")); + assert!(big_o_normal_form(&e).is_err()); +} + +#[test] +fn test_big_o_exp_dominates_polynomial() { + // 2^n + n^3 → O(2^n) + let e = Expr::parse("2^n + n^3"); + let result = big_o_normal_form(&e).unwrap(); + let s = result.to_string(); + assert!(s.contains("2^n"), "expected 2^n to survive, got: {s}"); + assert!( + !s.contains("n^3"), + "n^3 should be dominated by 2^n, got: {s}" + ); +} + +#[test] +fn test_big_o_larger_base_exp_dominates() { + // 3^n + 2^n → O(3^n) + let e = Expr::parse("3^n + 2^n"); + let result = big_o_normal_form(&e).unwrap(); + let s = result.to_string(); + assert!(s.contains("3^n"), "expected 3^n to survive, got: {s}"); + assert!( + !s.contains("2^n"), + "2^n should be dominated by 3^n, got: {s}" + ); +} + +#[test] +fn test_big_o_poly_log_dominates_poly() { + // n*log(n) + n → O(n*log(n)) + let e = Expr::parse("n * log(n) + n"); + let result = big_o_normal_form(&e).unwrap(); + let s = result.to_string(); + assert!(s.contains("log"), "expected n*log(n) to survive, got: {s}"); + assert!( + !s.ends_with("+ n") && !s.starts_with("n +"), + "bare n should be dominated by n*log(n), got: {s}" + ); +} + +#[test] +fn test_big_o_higher_poly_dominates_poly_log() { + // n^2 + n*log(n) → O(n^2) + let e = Expr::parse("n^2 + n * log(n)"); + let result = big_o_normal_form(&e).unwrap(); + assert_eq!(result.to_string(), "n^2"); +} + +#[test] +fn test_big_o_log_dominates_loglog() { + // log(n) + log(log(n)) → O(log(n)) + let e = Expr::parse("log(n) + log(log(n))"); + let result = big_o_normal_form(&e).unwrap(); + assert_eq!(result.to_string(), "log(n)"); +} + +#[test] +fn test_big_o_poly_dominates_log() { + // n + log(n) → O(n) + let e = Expr::parse("n + log(n)"); + let result = big_o_normal_form(&e).unwrap(); + assert_eq!(result.to_string(), "n"); +} + +#[test] +fn test_big_o_exp_n_dominates_two_n() { + // exp(n) dominates 2^n (since e > 2) + let e = Expr::parse("2^n + exp(n)"); + let result = big_o_normal_form(&e).unwrap(); + let s = result.to_string(); + assert!(s.contains("exp"), "expected exp(n) to survive, got: {s}"); + assert!( + !s.contains("2^n"), + "2^n should be dominated by exp(n), got: {s}" + ); +} + +#[test] +fn test_big_o_multivar_exp_dominates_poly() { + // 2^n + n * m → O(2^n) when n is in both + let e = Expr::parse("2^n + n * m"); + let result = big_o_normal_form(&e).unwrap(); + let s = result.to_string(); + // n*m has vars {n,m}, 2^n has vars {n}. n*m's vars are NOT a subset of 2^n's vars, + // so they're incomparable — both should survive. + assert!(s.contains("2^n"), "expected 2^n to survive, got: {s}"); + assert!( + s.contains("m"), + "expected n*m to survive (different var set), got: {s}" + ); +} diff --git a/src/unit_tests/canonical.rs b/src/unit_tests/canonical.rs index 8acb7e437..f2ce3ce1a 100644 --- a/src/unit_tests/canonical.rs +++ b/src/unit_tests/canonical.rs @@ -84,6 +84,36 @@ fn test_canonical_division_becomes_negative_exponent() { assert!((c.eval(&size) - 2.0).abs() < 1e-10); } +#[test] +fn test_canonical_distinct_fractional_exponents_do_not_merge() { + let e = Expr::pow(Expr::Var("n"), Expr::Const(1.0004)) - Expr::Var("n"); + let c = canonical_form(&e).unwrap(); + assert_ne!(c.to_string(), "0"); + let size = crate::types::ProblemSize::new(vec![("n", 2)]); + assert_ne!(c.eval(&size), 0.0); +} + +#[test] +fn test_canonical_constant_base_one_folds_to_constant() { + let e = Expr::pow(Expr::Const(1.0), Expr::Var("n")); + let c = canonical_form(&e).unwrap(); + assert_eq!(c.to_string(), "1"); +} + +#[test] +fn test_canonical_negative_constant_base_with_symbolic_exponent_is_rejected() { + let e = Expr::pow(Expr::Const(-2.0), Expr::Var("n")); + let err = canonical_form(&e).unwrap_err(); + assert!(matches!(err, CanonicalizationError::Unsupported(_))); +} + +#[test] +fn test_canonical_zero_constant_base_with_symbolic_exponent_is_rejected() { + let e = Expr::pow(Expr::Const(0.0), Expr::Var("n")); + let err = canonical_form(&e).unwrap_err(); + assert!(matches!(err, CanonicalizationError::Unsupported(_))); +} + #[test] fn test_canonical_deterministic_order() { // m + n and n + m should produce the same canonical form diff --git a/src/unit_tests/expr.rs b/src/unit_tests/expr.rs index 9c1b70f1d..a89c04256 100644 --- a/src/unit_tests/expr.rs +++ b/src/unit_tests/expr.rs @@ -249,11 +249,12 @@ fn test_asymptotic_normal_form_substitution_is_closed() { #[test] fn test_asymptotic_normal_form_handles_subtraction() { - // n - m now succeeds: canonical form gives both terms, both survive in Big-O - let result = asymptotic_normal_form(&Expr::parse("n - m")).unwrap(); - let s = result.to_string(); - assert!(s.contains("m"), "expected m in result, got: {s}"); - assert!(s.contains("n"), "expected n in result, got: {s}"); + // n - m: the -m term survives as a negative dominant term → unsupported + assert!(asymptotic_normal_form(&Expr::parse("n - m")).is_err()); + + // n^2 - n: -n is dominated by n^2 and eliminated → works + let result = asymptotic_normal_form(&Expr::parse("n^2 - n")).unwrap(); + assert_eq!(result.to_string(), "n^2"); } #[test] @@ -597,10 +598,10 @@ fn test_parse_precedence_mul_pow() { #[test] fn test_parse_precedence_unary_pow() { - // In our parser, unary minus binds tighter than ^: -n^2 = (-n)^2 - assert_eq!(parse_eval("-n^2", &[("n", 3)]), 9.0); - // Use parens for math convention: -(n^2) = -9 + // Unary minus binds less tightly than ^: -n^2 = -(n^2) + assert_eq!(parse_eval("-n^2", &[("n", 3)]), -9.0); assert_eq!(parse_eval("-(n^2)", &[("n", 3)]), -9.0); + assert_eq!(parse_eval("(-n)^2", &[("n", 3)]), 9.0); } // -- Error cases --