Implement Sub trait for LinearMonomial, QuadraticMonomial, and MonomialDyn#621
Implement Sub trait for LinearMonomial, QuadraticMonomial, and MonomialDyn#621
Conversation
…alDyn Add comprehensive subtraction operations for monomial types: - Add Sub trait implementations for monomial-to-monomial operations - Add Sub trait implementations for monomial-coefficient operations - Add Sub trait implementations for monomial-polynomial operations - Add missing PolynomialBase<M> - Coefficient operations - Support all reference and owned value combinations This enables subtraction operations like `linear!(1) - linear!(2)` that were previously causing compilation errors. All existing tests pass and documentation examples work correctly. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Create generic `impl_monomial_op!` macro that accepts operation type as parameter - Reduce ~170 lines of duplicated code between Add and Sub implementations - Fix comment: "Add Sub<Coefficient> operations" instead of "Add missing..." - Maintain backward compatibility with legacy macros delegating to generic one Addresses review comments from PR #621 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove unused $op_assign_trait and $op_assign_method parameters from impl_monomial_op macro - Update comments from "Add operations" to "Operations" for clarity - Fix misleading comments about Add-specific optimizations to reflect both Add/Sub operations - Update legacy macros to pass correct number of parameters 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive subtraction operations for monomial types in the Rust polynomial library. It adds Sub trait implementations across all monomial types (LinearMonomial, QuadraticMonomial, and MonomialDyn) with support for monomial-to-monomial, monomial-coefficient, and monomial-polynomial operations, including all reference and owned value combinations.
- Refactored existing add operations into a generic macro system that supports both Add and Sub traits
- Added missing PolynomialBase - Coefficient operations that were previously unavailable
- Implemented Sub trait for all three monomial types with comprehensive operation support
| (@internal Add, add, $self:expr, $rhs:expr) => {{ | ||
| let mut rhs = $rhs; | ||
| rhs.add_term($self, coeff!(1.0)); | ||
| rhs | ||
| }}; | ||
|
|
||
| // Internal helper for Sub implementation | ||
| (@internal Sub, sub, $self:expr, $rhs:expr) => {{ | ||
| let mut result = PolynomialBase::from($self); | ||
| result -= $rhs; | ||
| result | ||
| }}; |
There was a problem hiding this comment.
[nitpick] The Add and Sub implementations use different approaches: Add mutates the RHS polynomial directly while Sub creates a new polynomial from the monomial. This inconsistency could lead to confusion and different performance characteristics. Consider using a consistent approach for both operations.
| macro_rules! impl_monomial_add { | ||
| ($monomial:ty) => { | ||
| impl_monomial_op!(Add, add, $monomial); | ||
| }; | ||
| } | ||
|
|
||
| // Legacy macro for backward compatibility - delegates to generic macro | ||
| macro_rules! impl_monomial_sub { | ||
| ($monomial:ty) => { | ||
| impl_monomial_op!(Sub, sub, $monomial); | ||
| }; | ||
| } |
There was a problem hiding this comment.
[nitpick] The impl_monomial_add and impl_monomial_sub macros are now redundant wrapper macros that only delegate to impl_monomial_op. Consider removing these wrapper macros and calling impl_monomial_op! directly at the call sites to reduce macro indirection.
Add comprehensive subtraction operations for monomial types:
This enables subtraction operations like
linear!(1) - linear!(2)that were previously causing compilation errors.
All existing tests pass and documentation examples work correctly.
🤖 Generated with Claude Code
Co-Authored-By: Claude noreply@anthropic.com