-
Notifications
You must be signed in to change notification settings - Fork 112
Fix typo in QP problem instantiation #758
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
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughThis PR introduces new diving heuristics for branch-and-bound solving, refactors the dual simplex solver to use device-resident data, adds Quadratic Programming support to documentation, upgrades GitHub Actions workflows, updates Python APIs by removing deprecated parameters, and adds S3-based dataset downloads. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
93d9444 to
3c03eaf
Compare
Signed-off-by: mtanneau <mathieu.tanneau@gmail.com>
3c03eaf to
923bdf1
Compare
|
Thank you @mtanneau for the PR. Will get back to you on your suggestion on variable types. |
|
/ok to test f8d0336 |
|
Thank you @mtanneau for the PR, will ping engine team regarding your suggestion on removing. |
|
/ok to test f8d0336 |
|
/ok to test 740b221 |
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c (1)
105-108: Remove unusedvariable_typesdefinition.The
variable_typesarray is now dead code since it's no longer passed tocuOptCreateQuadraticProblem. As the PR author noted, this should be removed to avoid confusing users who might wonder why the variable exists but isn't used.♻️ Suggested fix
- // Variable types (continuous) - // From the constraints: - // x1, x2 >= 0 - char variable_types[] = {CUOPT_CONTINUOUS, CUOPT_CONTINUOUS}; - cuopt_int_t status;
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
🧰 Additional context used
📓 Path-based instructions (1)
docs/**/*
⚙️ CodeRabbit configuration file
docs/**/*: For documentation changes, focus on:
- Accuracy: Verify code examples compile and run correctly
- Completeness: Check if API changes (parameters, return values, errors) are documented
- Clarity: Flag confusing explanations, missing prerequisites, or unclear examples
- Consistency: Version numbers, parameter types, and terminology match code
- Examples: Suggest adding examples for complex features or new APIs
- Missing docs: If PR changes public APIs without updating docs, flag as HIGH priority
When code changes affect docs:
- Suggest specific doc files that need updates (e.g., docs/cuopt/api.rst)
- Identify outdated information contradicting the code changes
- Recommend documenting performance characteristics, GPU requirements, or numerical tolerances
Files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
🧠 Learnings (13)
📓 Common learnings
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/tests/linear_programming/c_api_tests/c_api_test.c:1033-1048
Timestamp: 2025-12-06T00:22:48.638Z
Learning: In cuOPT's quadratic programming API, when a user provides a quadratic objective matrix Q via set_quadratic_objective_matrix or the C API functions cuOptCreateQuadraticProblem/cuOptCreateQuadraticRangedProblem, the API internally computes Q_symmetric = Q + Q^T and the barrier solver uses 0.5 * x^T * Q_symmetric * x. From the user's perspective, the convention is x^T Q x. For a diagonal Q with values [q1, q2, ...], the resulting quadratic terms are q1*x1^2 + q2*x2^2 + ...
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Learnt from: aliceb-nv
Repo: NVIDIA/cuopt PR: 527
File: cpp/src/mip/diversity/lns/rins.cu:167-175
Timestamp: 2025-10-22T14:25:22.899Z
Learning: In MIP (Mixed Integer Programming) problems in the cuOPT codebase, `n_integer_vars == 0` is impossible by definition—MIP problems must have at least one integer variable. If there are no integer variables, it would be a pure Linear Programming (LP) problem, not a MIP problem.
📚 Learning: 2025-12-06T00:22:48.638Z
Learnt from: chris-maes
Repo: NVIDIA/cuopt PR: 500
File: cpp/tests/linear_programming/c_api_tests/c_api_test.c:1033-1048
Timestamp: 2025-12-06T00:22:48.638Z
Learning: In cuOPT's quadratic programming API, when a user provides a quadratic objective matrix Q via set_quadratic_objective_matrix or the C API functions cuOptCreateQuadraticProblem/cuOptCreateQuadraticRangedProblem, the API internally computes Q_symmetric = Q + Q^T and the barrier solver uses 0.5 * x^T * Q_symmetric * x. From the user's perspective, the convention is x^T Q x. For a diagonal Q with values [q1, q2, ...], the resulting quadratic terms are q1*x1^2 + q2*x2^2 + ...
Applied to files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Track GPU device memory allocations and deallocations to prevent memory leaks; ensure cudaMalloc/cudaFree balance and cleanup of streams/events
Applied to files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Check that hard-coded GPU device IDs and resource limits are made configurable; abstract multi-backend support for different CUDA versions
Applied to files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure race conditions are absent in multi-GPU code and multi-threaded server implementations; verify proper synchronization of shared state
Applied to files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cpp,hpp,h} : Avoid inappropriate use of exceptions in performance-critical GPU operation paths; prefer error codes or CUDA error checking for latency-sensitive code
Applied to files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify error propagation from CUDA to user-facing APIs is complete; ensure CUDA errors are caught and mapped to meaningful user error codes
Applied to files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Eliminate unnecessary host-device synchronization (cudaDeviceSynchronize) in hot paths that blocks GPU pipeline; use streams and events for async execution
Applied to files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Verify correct problem size checks before expensive GPU/CPU operations; prevent resource exhaustion on oversized problems
Applied to files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Refactor code duplication in solver components (3+ occurrences) into shared utilities; for GPU kernels, use templated device functions to avoid duplication
Applied to files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.cu : Verify race conditions and correctness of GPU kernel shared memory, atomics, and warp-level operations
Applied to files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : For concurrent CUDA operations (barriers, async operations), explicitly create and manage dedicated streams instead of reusing the default stream; document stream lifecycle
Applied to files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
📚 Learning: 2025-11-25T10:20:49.822Z
Learnt from: CR
Repo: NVIDIA/cuopt PR: 0
File: .github/.coderabbit_review_guide.md:0-0
Timestamp: 2025-11-25T10:20:49.822Z
Learning: Applies to **/*.{cu,cuh,cpp,hpp,h} : Ensure variables and constraints are accessed from the correct problem context (original vs presolve vs folded vs postsolve); verify index mapping consistency across problem transformations
Applied to files:
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c
🔇 Additional comments (2)
docs/cuopt/source/cuopt-c/lp-milp/examples/simple_qp_example.c (2)
2-2: LGTM!Copyright year update is appropriate.
118-133: LGTM!The fix correctly removes the
variable_typesargument from thecuOptCreateQuadraticProblemcall, which resolves the compilation error reported in issue #757. The function signature now matches the API that doesn't accept variable types for QP problems.
|
/ok to test 1a0747c |
Description
Fixes #757 by removing extra argument when creating QP problem instance.
Note that since the QP problem constructor does not accept variable types, one can remove the definition of variable types which are unused. Happy to do that if preferable :)
Issue
closes #757
Checklist
Summary by CodeRabbit
Release Notes
New Features
Improvements
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.