Skip to content

Conversation

kwesiRutledge
Copy link
Contributor

No description provided.

@kwesiRutledge kwesiRutledge requested a review from Copilot August 5, 2025 20:17
@kwesiRutledge kwesiRutledge self-assigned this Aug 5, 2025
Copy link

@Copilot 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 optimizes the ToProblemWithAllPositiveVariables function to avoid creating unnecessary variables when transforming an optimization problem. The optimization checks if positive and negative parts of variables are actually needed based on variable bounds and constraint redundancy before creating them.

Key Changes:

  • Added logic to conditionally create positive and negative variable parts only when necessary
  • Introduced redundancy checking using ConstraintIsRedundantGivenOthers function
  • Added a new test case to verify the optimization works correctly

Reviewed Changes

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

File Description
problem/optimization_problem.go Optimizes variable creation logic and removes commented code
testing/problem/optimization_problem_test.go Adds test case for the optimization scenario
Comments suppressed due to low confidence (1)

problem/optimization_problem.go:585

  • The variable name 'epsMagic' is unclear and unprofessional. Consider renaming it to something more descriptive like 'epsilon' or 'toleranceThreshold'.
	epsMagic := 1e-8 // TODO(Kwesi): Make this a parameter OR a constant in the package.

variablePositivePart := newProblem.Variables[nVariables-1]
// - Positive Part
positivePartExists := xII.Upper >= 0
positivePartExists = positivePartExists && !ConstraintIsRedundantGivenOthers(xII.LessEq(0.0-epsMagic), op.Constraints)
Copy link
Preview

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

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

The expression '0.0-epsMagic' is unclear. Consider using '-epsMagic' for better readability.

Suggested change
positivePartExists = positivePartExists && !ConstraintIsRedundantGivenOthers(xII.LessEq(0.0-epsMagic), op.Constraints)
positivePartExists = positivePartExists && !ConstraintIsRedundantGivenOthers(xII.LessEq(-epsMagic), op.Constraints)

Copilot uses AI. Check for mistakes.

Copy link

codecov bot commented Aug 5, 2025

Codecov Report

❌ Patch coverage is 96.63866% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.08%. Comparing base (0214315) to head (4a1f585).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
problem/optimization_problem.go 93.10% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #12      +/-   ##
==========================================
+ Coverage   86.76%   87.08%   +0.32%     
==========================================
  Files          27       32       +5     
  Lines        3687     4136     +449     
==========================================
+ Hits         3199     3602     +403     
- Misses        440      480      +40     
- Partials       48       54       +6     

☔ 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.

@kwesiRutledge kwesiRutledge merged commit c242cba into main Aug 5, 2025
5 checks passed
@kwesiRutledge kwesiRutledge deleted the kr/feature/smart-all-positive-variable-creation1 branch August 5, 2025 21:03
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.

1 participant