Skip to content

Replace Polynomial overhead system with Expr AST#99

Merged
isPANN merged 15 commits intomainfrom
feat/expr-overhead-system
Feb 26, 2026
Merged

Replace Polynomial overhead system with Expr AST#99
isPANN merged 15 commits intomainfrom
feat/expr-overhead-system

Conversation

@GiggleLiu
Copy link
Contributor

Summary

  • Introduce Expr AST type (src/expr.rs) replacing Polynomial for reduction overhead expressions
  • Add Pratt expression parser in proc macro crate for compile-time parsing of overhead strings
  • New #[reduction] syntax: overhead = { field = "expression" } instead of inline poly!() calls
  • Add inherent size getter methods to all 21 problem types
  • Remove problem_size_names()/problem_size_values() from Problem trait
  • Remove source_size_names_fn/target_size_names_fn from ReductionEntry
  • Delete Polynomial type and poly! macro entirely
  • Update CLI, MCP server, and documentation

Test plan

  • All 1,613 tests pass (1,474 unit + 96 integration + 43 doc)
  • make check (fmt + clippy + test) passes
  • make mcp-test passes
  • make examples and make export-schemas succeed
  • No new clippy warnings

🤖 Generated with Claude Code

GiggleLiu and others added 12 commits February 25, 2026 23:21
Macro-first dual emission approach: compile-time parsed expression
strings emit both Rust getter-calling code and symbolic Expr AST
for composition/export.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Detailed TDD-style implementation plan for issue #61.
Phases: Expr type → macro parser → getters → migration → cleanup → docs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…verhead syntax

- Update #[reduction] macro to detect and parse new `name = "expr"` syntax
- Switch ReductionOverhead to store Expr instead of Polynomial
- Add From<Polynomial> for Expr bridge conversion
- Add scale() and Add impl to Expr
- Update poly! macro to produce Expr via conversion
- Update export format to use expr/formula fields
- Update factoring_ilp and travelingsalesman_ilp to use from_polynomials
- Update all affected tests

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add num_vertices(), num_edges(), num_vars(), num_constraints(),
num_literals(), universe_size(), num_interactions(), num_variables(),
num_bits_first(), num_bits_second(), num_sequence(), rank(), m(), n()
getters matching problem_size_names() fields.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace poly!() macro calls with string-based overhead expressions
across 33 reduction files. The new syntax uses `field = "expr"` format
parsed at compile time by the Pratt expression parser.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove problem_size_names() and problem_size_values() from the Problem
trait and all 21 model implementations. Remove source_size_names_fn and
target_size_names_fn from ReductionEntry. Derive size field names from
overhead expressions instead. Update CLI dispatch and MCP tools.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Delete src/polynomial.rs and its tests. Remove From<Polynomial> impl
on Expr and from_polynomials() bridge on ReductionOverhead. Update
remaining registry tests to use Expr directly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fix formatting, suppress dead_code warning in parser, update CLAUDE.md
and design.md to reflect removal of problem_size_names/values and
addition of Expr-based overhead system.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…type

Fix borrow-after-move in graph show command and use reference in
MCP tools JSON serialization.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this file.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR replaces the Polynomial-based reduction overhead system with a more general Expr AST representation. The new system uses compile-time expression parsing via a Pratt parser in the proc macro, replacing the runtime-only poly! macro and trait-based problem_size_names()/problem_size_values() methods with inherent getters on problem types.

Changes:

  • Introduced Expr AST type with support for arithmetic operations, exponentiation, exp, log, and sqrt
  • Added Pratt parser in proc macro crate for compile-time parsing of overhead expression strings
  • Replaced #[reduction(overhead = { ReductionOverhead::new(...) })] syntax with overhead = { field = "expression" }
  • Added inherent size getter methods (e.g., num_vertices(), num_edges()) to all 21 problem types
  • Removed problem_size_names()/problem_size_values() from Problem trait and all implementations
  • Deleted entire Polynomial type, Monomial type, and poly! macro
  • Updated export schema to include both AST (expr) and display string (formula) fields

Reviewed changes

Copilot reviewed 85 out of 85 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/expr.rs New Expr AST enum with eval, substitute, variables, and Display
problemreductions-macros/src/parser.rs New Pratt parser for overhead expression strings
problemreductions-macros/src/lib.rs Updated #[reduction] macro to support new syntax alongside legacy
src/models/**/*.rs Added inherent getter methods, removed problem_size_* trait methods from all 21 problem types
src/rules/**/*.rs Migrated all reductions to new overhead syntax
src/rules/registry.rs Changed ReductionOverhead to use Expr instead of Polynomial, removed size name fields from ReductionEntry
src/polynomial.rs Deleted entire file (340 lines)
src/unit_tests/polynomial.rs Deleted (180 tests)
src/unit_tests/expr.rs Added comprehensive tests for new Expr type
src/export.rs Updated schema: OverheadEntry now has expr and formula fields
problemreductions-cli/** Updated CLI and MCP to use size_field_names() from reduction graph
docs/** Updated design docs and CLAUDE.md to reflect new system

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

GiggleLiu and others added 2 commits February 26, 2026 09:47
size_field_names() was returning output_size field names from the first
matching reduction entry, which are the TARGET problem's fields when
the queried problem is a source. Now correctly extracts input variable
names when the problem is a source, and collects from all entries for
completeness.

Also tightens CLI inspect test to assert actual field names, and adds
a cross-check test verifying all overhead variables are valid source
size fields.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
**Date:** 2026-02-25
**Approach:** Macro-first dual emission

## Summary
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isPANN Please carefully check and verify this design document. Any thoughts?

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov
Copy link

codecov bot commented Feb 26, 2026

Codecov Report

❌ Patch coverage is 70.57292% with 113 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.46%. Comparing base (9671215) to head (cc8bebb).
⚠️ Report is 16 commits behind head on main.

Files with missing lines Patch % Lines
src/expr.rs 77.65% 21 Missing ⚠️
src/models/set/maximum_set_packing.rs 0.00% 7 Missing ⚠️
src/models/graph/kcoloring.rs 0.00% 6 Missing ⚠️
src/models/graph/max_cut.rs 0.00% 6 Missing ⚠️
src/models/graph/maximal_is.rs 0.00% 6 Missing ⚠️
src/models/graph/maximum_clique.rs 0.00% 6 Missing ⚠️
src/models/graph/maximum_independent_set.rs 0.00% 6 Missing ⚠️
src/models/graph/maximum_matching.rs 0.00% 6 Missing ⚠️
src/models/graph/minimum_dominating_set.rs 0.00% 6 Missing ⚠️
src/models/graph/minimum_vertex_cover.rs 0.00% 6 Missing ⚠️
... and 9 more

❌ Your patch status has failed because the patch coverage (70.57%) is below the target coverage (95.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #99      +/-   ##
==========================================
+ Coverage   96.36%   96.46%   +0.10%     
==========================================
  Files         197      196       -1     
  Lines       27214    26620     -594     
==========================================
- Hits        26224    25679     -545     
+ Misses        990      941      -49     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@isPANN isPANN merged commit f1d9cc9 into main Feb 26, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants