-
Couldn't load subscription status.
- Fork 86
Add support for quadratic programming to the barrier method #500
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
This PR extends cuOPT's barrier method solver to support quadratic programming (QP) problems alongside the existing linear programming (LP) functionality. The implementation adds storage and handling for a quadratic objective matrix Q throughout the optimization pipeline. The changes introduce three new vector fields (Q_offsets, Q_indices, Q_values) to store the quadratic matrix in compressed sparse format, following the same pattern used for constraint matrices. The quadratic objective function takes the form (1/2)x^T Q x + c^T x, enabling the solver to handle more complex optimization problems like portfolio optimization and machine learning applications.
The implementation modifies the barrier method's interior point algorithm to properly handle quadratic terms in the KKT system. Key architectural changes include updating the augmented system formation to include Q matrix entries, modifying dual residual computations to incorporate Q*x terms, and ensuring step size consistency for quadratic problems. The changes maintain backward compatibility by treating empty Q matrices as linear problems.
Important Files Changed
Changed Files
| Filename | Score | Overview |
|---|---|---|
cpp/src/dual_simplex/barrier.cu |
4/5 | Implements core quadratic programming support in barrier method with Q matrix handling, KKT system modifications, and residual computations |
cpp/include/cuopt/linear_programming/optimization_problem.hpp |
4/5 | Adds API for quadratic objective matrix storage with setter and getter methods |
cpp/src/linear_programming/optimization_problem.cu |
4/5 | Implements quadratic matrix storage methods and debug output functionality |
cpp/src/dual_simplex/presolve.cpp |
4/5 | Adds quadratic matrix initialization and disables incompatible optimizations for QP problems |
cpp/src/mip/problem/problem.cu |
4/5 | Integrates quadratic objective components into MIP problem constructors |
cpp/src/linear_programming/solve.cu |
4/5 | Connects MPS quadratic data parsing to optimization problem structure |
cpp/src/mip/problem/problem.cuh |
4/5 | Declares quadratic matrix storage vectors in problem class header |
cpp/src/dual_simplex/presolve.hpp |
4/5 | Adds Q matrix field to lp_problem_t structure for quadratic programming |
cpp/src/dual_simplex/user_problem.hpp |
5/5 | Adds quadratic matrix storage vectors to user problem structure |
cpp/src/linear_programming/translate.hpp |
4/5 | Transfers quadratic matrix data between problem structures with debug output |
cpp/libmps_parser/src/mps_parser.cpp |
4/5 | Adds debug output for quadratic objective matrix parsing |
cpp/src/dual_simplex/scaling.cpp |
3/5 | Disables column scaling for quadratic problems but contains syntax error in logical operator |
Confidence score: 4/5
- This PR appears safe to merge with moderate risk due to the comprehensive nature of the changes and good architectural integration
- Score reflects the extensive modifications across multiple solver components and the presence of debug code/syntax issues that should be addressed
- Pay close attention to
cpp/src/dual_simplex/scaling.cppwhich has a logical operator syntax error, andcpp/src/dual_simplex/barrier.cudue to its complexity and central role in QP functionality
Sequence Diagram
sequenceDiagram
participant User
participant MPS_Parser as mps_parser_t
participant Problem as optimization_problem_t
participant Barrier as barrier_solver_t
User->>MPS_Parser: Parse MPS file with QUADOBJ section
MPS_Parser->>MPS_Parser: parse_quadobj(line)
Note over MPS_Parser: Parse quadratic objective entries<br/>(upper triangular format)
MPS_Parser->>MPS_Parser: Store quadobj_entries as tuples<br/>(var1_id, var2_id, value)
MPS_Parser->>MPS_Parser: fill_problem(data_model)
Note over MPS_Parser: Convert to CSR format using<br/>double transpose method
MPS_Parser->>MPS_Parser: build_csr_via_transpose(quadobj_entries)
Note over MPS_Parser: Expand upper triangular to<br/>full symmetric matrix
MPS_Parser->>Problem: set_quadratic_objective_matrix(Q_values, Q_indices, Q_offsets)
Problem->>Problem: Store Q_offsets_, Q_indices_, Q_values_
User->>Problem: solve_lp()
Problem->>Problem: cuopt_problem_to_simplex_problem()
Note over Problem: Convert problem structure<br/>for dual simplex format
Problem->>Problem: Copy Q data to user_problem
Note over Problem: user_problem.Q_offsets = Q_offsets<br/>user_problem.Q_indices = Q_indices<br/>user_problem.Q_values = Q_values
Problem->>Barrier: solve_linear_program_with_barrier()
Barrier->>Barrier: Create lp_problem_t with Q matrix
Note over Barrier: Q.m = lp.num_cols<br/>Q.n = lp.num_cols<br/>Convert row format to column format
Barrier->>Barrier: form_augmented(Q matrix)
Note over Barrier: Build augmented system with<br/>quadratic terms: -Q - diag - dual_perturb
Barrier->>Barrier: Initialize barrier method
Note over Barrier: Handle quadratic programming<br/>with interior point method
Barrier-->>Problem: Return solution
Problem-->>User: Return optimized solution
12 files reviewed, 17 comments
| printf("user_problem.Q_offsets: %zu\n", user_problem.Q_offsets.size()); | ||
| printf("user_problem.Q_indices: %zu\n", user_problem.Q_indices.size()); | ||
| printf("user_problem.Q_values: %zu\n", user_problem.Q_values.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Debug printf statements should be removed before merging - use logging instead
| printf("user_problem.Q_offsets: %zu\n", user_problem.Q_offsets.size()); | |
| printf("user_problem.Q_indices: %zu\n", user_problem.Q_indices.size()); | |
| printf("user_problem.Q_values: %zu\n", user_problem.Q_values.size()); | |
| CUOPT_LOG_DEBUG("user_problem.Q_offsets: {}", user_problem.Q_offsets.size()); | |
| CUOPT_LOG_DEBUG("user_problem.Q_indices: {}", user_problem.Q_indices.size()); | |
| CUOPT_LOG_DEBUG("user_problem.Q_values: {}", user_problem.Q_values.size()); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: cpp/src/linear_programming/translate.hpp
Line: 112:114
Comment:
**style:** Debug printf statements should be removed before merging - use logging instead
```suggestion
CUOPT_LOG_DEBUG("user_problem.Q_offsets: {}", user_problem.Q_offsets.size());
CUOPT_LOG_DEBUG("user_problem.Q_indices: {}", user_problem.Q_indices.size());
CUOPT_LOG_DEBUG("user_problem.Q_values: {}", user_problem.Q_values.size());
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| // Process QUADOBJ data if present (upper triangular format) | ||
| if (!quadobj_entries.empty()) { | ||
| printf("Quadratic objective matrix: %zu nonzeros\n", quadobj_entries.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider using proper logging instead of printf for consistency with the rest of the codebase
Prompt To Fix With AI
This is a comment left during a code review.
Path: cpp/libmps_parser/src/mps_parser.cpp
Line: 509:509
Comment:
**style:** Consider using proper logging instead of `printf` for consistency with the rest of the codebase
How can I resolve this? If you propose a fix, please make it concise.| /** Quadratic objective matrix in CSR format (for (1/2) * x^T * Q * x term) */ | ||
| std::vector<i_t> Q_offsets_; | ||
| std::vector<i_t> Q_indices_; | ||
| std::vector<f_t> Q_values_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Quadratic matrix stored as host std::vectors while other matrices use rmm::device_uvector. This creates inconsistency in memory management - consider if device storage is needed for GPU operations.
Prompt To Fix With AI
This is a comment left during a code review.
Path: cpp/include/cuopt/linear_programming/optimization_problem.hpp
Line: 435:438
Comment:
**style:** Quadratic matrix stored as host std::vectors while other matrices use rmm::device_uvector. This creates inconsistency in memory management - consider if device storage is needed for GPU operations.
How can I resolve this? If you propose a fix, please make it concise.| const f_t* Q_values, | ||
| i_t size_values) | ||
| { | ||
| printf("optimization_problem_t Quadratic objective matrix: %d nonzeros\n", size_values); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Debug printf should be removed or replaced with proper logging using CUOPT_LOG_DEBUG or similar logging infrastructure
Prompt To Fix With AI
This is a comment left during a code review.
Path: cpp/src/linear_programming/optimization_problem.cu
Line: 154:154
Comment:
**style:** Debug printf should be removed or replaced with proper logging using CUOPT_LOG_DEBUG or similar logging infrastructure
How can I resolve this? If you propose a fix, please make it concise.| i_t factorization_size = n + m; | ||
| const f_t dual_perturb = 0.0; | ||
| const f_t primal_perturb = 1e-6; | ||
| const f_t primal_perturb = 1e-12; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider using a more robust perturbation value than 1e-12 for the primal perturbation, especially for ill-conditioned problems
Prompt To Fix With AI
This is a comment left during a code review.
Path: cpp/src/dual_simplex/barrier.cu
Line: 366:366
Comment:
**style:** Consider using a more robust perturbation value than 1e-12 for the primal perturbation, especially for ill-conditioned problems
How can I resolve this? If you propose a fix, please make it concise.| settings_.log.printf("augmented nnz %d predicted %d\n", q, 2 * nnzA + n + m + off_diag_Qnz); | ||
| exit(1); | ||
| } | ||
| if (A.col_start[n] != AT.col_start[m]) { | ||
| settings_.log.printf("A nz %d AT nz %d\n", A.col_start[n], AT.col_start[m]); | ||
| exit(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Using exit(1) for assertion failures can be problematic in library code - consider returning error codes or throwing exceptions instead
Prompt To Fix With AI
This is a comment left during a code review.
Path: cpp/src/dual_simplex/barrier.cu
Line: 417:422
Comment:
**style:** Using `exit(1)` for assertion failures can be problematic in library code - consider returning error codes or throwing exceptions instead
How can I resolve this? If you propose a fix, please make it concise.
cpp/src/dual_simplex/barrier.cu
Outdated
| if (data.Q.n > 0) { | ||
| matrix_vector_multiply(data.Q, -1.0, data.x, 1.0, c); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: The modification of vector c here creates a temporary that includes Q*x. Consider documenting this side effect or using a separate variable name for clarity
Prompt To Fix With AI
This is a comment left during a code review.
Path: cpp/src/dual_simplex/barrier.cu
Line: 1708:1710
Comment:
**style:** The modification of vector `c` here creates a temporary that includes Q*x. Consider documenting this side effect or using a separate variable name for clarity
How can I resolve this? If you propose a fix, please make it concise.|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Add support for quadratic programming to the barrier method.
Issue
Checklist