Skip to content

Remove wrong exception of unsupported QCQP#1493

Open
rg20 wants to merge 4 commits into
NVIDIA:mainfrom
rg20:qcqp_remove_exception
Open

Remove wrong exception of unsupported QCQP#1493
rg20 wants to merge 4 commits into
NVIDIA:mainfrom
rg20:qcqp_remove_exception

Conversation

@rg20

@rg20 rg20 commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

We were wrongly throwing an exception when the rotated cones and quadratic objectives were present.

Issue

Checklist

  • I am familiar with the Contributing Guidelines.
  • Testing
    • New or existing tests cover these changes
    • Added tests
    • Created an issue to follow-up
    • NA
  • Documentation
    • The documentation is up to date with these changes
    • Added new documentation
    • NA
@rg20 rg20 requested a review from a team as a code owner June 30, 2026 18:14
@rg20 rg20 requested review from kaatish and yuwenchen95 June 30, 2026 18:14
@copy-pr-bot

copy-pr-bot Bot commented Jun 30, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.
@rg20 rg20 removed the request for review from yuwenchen95 June 30, 2026 18:14
@rg20 rg20 added bug Something isn't working non-breaking Introduces a non-breaking change labels Jun 30, 2026
@rg20 rg20 added this to the 26.08 milestone Jun 30, 2026
@rg20 rg20 requested review from yuwenchen95 and removed request for kaatish June 30, 2026 18:16
@rg20

rg20 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test d789f4e
mlubin
mlubin reviewed Jun 30, 2026
View reviewed changes
Comment thread cpp/tests/socp/general_quadratic_test.cu Outdated
user_problem.var_types.assign(n, variable_type_t::CONTINUOUS);

// Rotated SOC: x^2 - 2*t*u <= 0
// Q COO: (0,0,1) for x^2, (1,2,-2) for -2*t*u (canonical form: single cross term)

@mlubin mlubin Jun 30, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try specifying the problem using the lp reader? I can't visually verify the correctness of the hardcoded matrix inputs.

@rg20 rg20 Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated
@github-actions

github-actions Bot commented Jun 30, 2026 •
edited
Loading

Copy link
Copy Markdown

CI Test Summary

3 failed · 28 passed · 0 skipped

conda-cpp-tests / 12.9.2, 3.14, amd64, ubuntu22.04, h100, latest-driver, latest-deps — 1 failed test
  • general_quadratic.qcqp_rotated_soc
conda-cpp-tests / 12.2.2, 3.11, amd64, rockylinux8, v100, earliest-driver, oldest-deps — 1 failed test
  • general_quadratic.qcqp_rotated_soc
conda-cpp-tests / 12.2.2, 3.11, arm64, ubuntu22.04, a100, latest-driver, latest-deps — 1 failed test
  • general_quadratic.qcqp_rotated_soc
yuwenchen95
yuwenchen95 reviewed Jul 1, 2026
View reviewed changes
Comment thread cpp/tests/socp/general_quadratic_test.cu Outdated
@rg20

rg20 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test ae2fea1
@coderabbitai

coderabbitai Bot commented Jul 1, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9c398ea4-8652-41de-8163-42ef3d3ace6c

📥 Commits

Reviewing files that changed from the base of the PR and between 52ca23a and ae2fea1.

📒 Files selected for processing (2)
  • cpp/src/barrier/translate_soc.hpp
  • cpp/tests/socp/general_quadratic_test.cu
💤 Files with no reviewable changes (1)
  • cpp/src/barrier/translate_soc.hpp
📝 Walkthrough

Walkthrough

Removes a validation guard in the rotated-SOC conversion path that previously rejected problems containing quadratic objective terms, allowing rotated SOC lifting to proceed regardless of Q_values. Adds a new unit test verifying a QCQP with a quadratic objective and rotated SOC constraint solves correctly.

Changes

Rotated SOC with quadratic objective support

Layer / File(s) Summary
Remove quadratic objective restriction
cpp/src/barrier/translate_soc.hpp		 Deletes the check requiring empty Q_values before rotated-SOC conversion, allowing the block to proceed with numeric setup and cone lifting even when quadratic objective terms exist.
Test QCQP with rotated SOC and quadratic objective
cpp/tests/socp/general_quadratic_test.cu		 Adds includes for pdlp_solver_settings_t and solve_lp, and introduces qcqp_rotated_soc test that parses a QCQP LP string, solves it via solve_lp, and asserts Optimal status with objective near -0.5.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: chris-maes, mlubin, ramakrishnap-nv

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: removing an incorrect unsupported-QCQP exception.
Description check ✅ Passed The description matches the bug fix and mentions the added tests, so it is clearly related to the changeset.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.
mlubin
mlubin approved these changes Jul 1, 2026
View reviewed changes
Comment thread cpp/tests/socp/general_quadratic_test.cu
ASSERT_EQ(problem.get_quadratic_constraints().size(), 1u);

pdlp_solver_settings_t<i_t, f_t> settings;
auto solution = solve_lp(&handle, problem, settings);

@mlubin mlubin Jul 1, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mind leaving a TODO to redo all the other tests in this form?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

3 participants

@rg20 @mlubin @yuwenchen95