Fix Bug in How LinearConstraint Representations handle w.r.t. variables for scalar case#17
Conversation
…ntations (make sure that we always include the wrt variable)
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes a bug in the handling of linear constraint representations for scalar and vector constraints while updating some related documentation comments for clarity.
- Fixes error message and constant mismatches in scalar and vector constraint tests
- Updates method signatures to pass the optional "wrt" variables for linear coefficient extraction
- Improves inline comments in vector and polynomial-like vector expression files
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| testing/symbolic/vector_constraint_test.go | Adds a new test for vector inequality constraints; note that an error message incorrectly references the equality representation. |
| testing/symbolic/scalar_constraint_test.go | Adds two new tests for scalar constraints; error messages reference an incorrect constant value in both inequality and equality cases. |
| symbolic/vector_expression.go | Updates the comment for the linear coefficient method; the comment name "LinearCoeffs" does not match the method name "LinearCoeff". |
| symbolic/variable.go | Adds support for handling mat.VecDense types by converting them into KVector structures. |
| symbolic/scalar_constraint.go | Updates the methods to pass the "wrt" parameter to the linear coefficient extraction methods. |
| symbolic/polynomial_like_vector.go | Updates the comment for the linear coefficient method; the comment name is inconsistent with the method name. |
| symbolic/constant_vector.go | Adds new cases in the addition and subtraction operations to handle additional types. |
Comments suppressed due to low confidence (5)
testing/symbolic/vector_constraint_test.go:597
- The error message incorrectly references 'LinearEqualityConstraintRepresentation' instead of 'LinearInequalityConstraintRepresentation'. Please update it to avoid confusion.
t.Errorf("Expected vc.LinearEqualityConstraintRepresentation()'s b vector to contain a 1 at the %v-th index; received %v", 0, b.AtVec(0))
testing/symbolic/scalar_constraint_test.go:694
- The error message expects a constant value of 2.5, but the test sets the constant to 2.1. Please correct the expected value in the error message.
t.Errorf("Expected b to be 2.5; received %v", b)
testing/symbolic/scalar_constraint_test.go:937
- The error message expects a constant value of 2.5, but the test uses 2.1 as the constant. Please update the expected value accordingly.
t.Errorf("Expected b to be 2.5; received %v", b)
symbolic/vector_expression.go:33
- [nitpick] The comment refers to 'LinearCoeffs' while the method is named 'LinearCoeff'. Consider updating the comment to match the method name for consistency.
// LinearCoeffs returns a slice of the coefficients in the expression
symbolic/polynomial_like_vector.go:33
- [nitpick] Ensure that the comment accurately reflects the method name and behavior; if the intended name is plural or singular, update it for consistency across the codebase.
// LinearCoeff returns a slice of the coefficients in the expression
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17 +/- ##
==========================================
- Coverage 86.83% 86.75% -0.08%
==========================================
Files 42 42
Lines 5916 5921 +5
==========================================
Hits 5137 5137
- Misses 709 714 +5
Partials 70 70 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Found a bug as soon as I tried to use the new features. 😭 This should fix it.