change semantics of problem construction in cuOptReadProblem

[tmckayus]

Since the mps data model is constructed internally, use move semantics when building a CPU problem to avoid an unnecessary copy. This is a small optimization but on a large problem with 10M variables and 200k constraints, it can save half a second in problem creation.

[coderabbitai]

📝 Walkthrough

Walkthrough

CPU-backed optimization problems now adopt parsed MPS/QPS storage without copying, while device-backed problems retain population fallback. Reader call sites use the new path, solution parsing uses problem-owned variable names, and tests compare adoption with population.

Changes

MPS Data Model Adoption

Layer / File(s)	Summary
Adoption contracts and backend dispatch cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp, cpp/include/cuopt/mathematical_optimization/optimization_problem_utils.hpp	Adds the CPU adoption API and dispatches CPU targets to move-based adoption while retaining population for other backends.
CPU model transfer and classification cpp/src/pdlp/cpu_optimization_problem.cpp	Moves model fields and quadratic constraints into CPU problems, converts variable types, derives problem categories, and resets the source model.
Reader integration and equivalence validation cpp/src/pdlp/cuopt_c.cpp, cpp/cuopt_cli.cpp, cpp/tests/linear_programming/unit_tests/solution_interface_test.cu	Updates readers to adopt parsed models, obtains solution variable names from the created problem, and verifies adoption matches population for linear and quadratic cases.

Estimated code review effort: 4 (Complex) | ~45 minutes

Suggested reviewers: kaatish, mlubin, akifcorduk, ramakrishnap-nv

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title references the changed problem-construction behavior in cuOptReadProblem, which matches the main move-semantics update.
Description check	✅ Passed	The description matches the PR’s optimization goal of using move semantics to avoid copying the internally created MPS data model.
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.}

[coderabbitai]

🧹 Nitpick comments (4)

cpp/tests/linear_programming/unit_tests/solution_interface_test.cu (1)

495-517: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

No test case exercises a non-empty quadratic objective through the adopt path.

expect_adopt_matches_populate already asserts on get_quadratic_objective_values/indices/offsets() (lines 471-473), but neither adopt_matches_populate_lp_file nor adopt_matches_populate_quadratic_constraints supplies a model with a quadratic objective — only quadratic constraints are covered. The Q_values_/Q_indices_/Q_offsets_ move branch in cpu_optimization_problem_t::adopt_from_mps_data_model (cpp_optimization_problem.cpp lines 1165-1167) is therefore untested against the populate baseline.

As per path instructions for cpp/tests/**, tests should validate numerical correctness for the specific paths under test rather than just "runs without error." Want me to draft a QP-objective test case (e.g. read_lp_from_string with a quadratic objective term) reusing expect_adopt_matches_populate?

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/tests/linear_programming/unit_tests/solution_interface_test.cu` around
lines 495 - 517, Add a test in SolutionInterfaceTest that builds a model with a
non-empty quadratic objective using read_lp_from_string, then passes it to
expect_adopt_matches_populate. Keep the existing quadratic-constraint coverage
and ensure the new case exercises the quadratic objective values, indices, and
offsets comparison through the adopt path.

Source: Path instructions

cpp/src/pdlp/cpu_optimization_problem.cpp (2)

1139-1183: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Adopt path bypasses the validating setters used by the populate path.

adopt_from_mps_data_model assigns A_, Q_values_/Q_indices_/Q_offsets_, etc. directly via std::move, skipping the cuopt_expects null/size checks that set_csr_constraint_matrix / set_quadratic_objective_matrix (and populate_from_mps_data_model) perform. This is presumably fine since mps_data_model_t is already internally consistent after parsing, but it means a parser defect would no longer be caught here and could surface later as an out-of-bounds read (e.g. indexing Q_offsets_[n_vars] downstream).

Consider adding lightweight invariant checks (e.g. cuopt_expects on offsets/size consistency) mirroring what the setters enforce, so a parser bug fails fast instead of propagating silently.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/pdlp/cpu_optimization_problem.cpp` around lines 1139 - 1183,
adopt_from_mps_data_model bypasses validation performed by the matrix setters,
allowing malformed CSR data to propagate. Add lightweight cuopt_expects
invariant checks in this method for A_ and Q_values_/Q_indices_/Q_offsets_
nullability, offset lengths, and index/value size consistency, mirroring
set_csr_constraint_matrix and set_quadratic_objective_matrix before downstream
use.

---

1114-1135: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

Use explicit field assignment here. The two quadratic_constraint_t layouts match today, but this positional aggregate init is coupled to both declarations and can break silently if either is reordered. add_quadratic_constraint already uses named assignments, so mirroring that pattern here would be safer.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/src/pdlp/cpu_optimization_problem.cpp` around lines 1114 - 1135, Replace
the positional aggregate initialization in move_quadratic_constraints_from_model
with explicit field-by-field assignments, mirroring the named-assignment pattern
used by add_quadratic_constraint. Populate each destination
quadratic_constraint_t member from qc, preserving move semantics for strings and
vectors, then append the fully initialized object to converted.

cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp (1)

85-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Doc comment missing @param tag.

The brief and moved-from note are good, but there's no @param describing data_model, unlike the free-function counterpart in optimization_problem_utils.hpp which documents @tparam/@param fully.

As per path instructions, "New public functions/classes need Doxygen-style documentation" and "Verify parameter descriptions match actual types/behavior" for public C++ headers under cpp/include/cuopt/**/*.

📝 Proposed doc addition

   /**
    * `@brief` Transfer parsed MPS/QPS storage into this CPU problem without copying array/string data.
    *
    * The model is left in a moved-from state and must not be used afterward.
+   *
+   * `@param`[in,out] data_model Parsed MPS/QPS model to adopt; left moved-from on return.
    */
   void adopt_from_mps_data_model(io::mps_data_model_t<i_t, f_t>&& data_model);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp`
around lines 85 - 91, Add a Doxygen `@param` description for the rvalue-reference
parameter data_model in adopt_from_mps_data_model, stating that it is the parsed
MPS/QPS storage being adopted without copying and is left moved-from afterward;
keep the documentation consistent with the corresponding free function.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp`:
- Around line 85-91: Add a Doxygen `@param` description for the rvalue-reference
parameter data_model in adopt_from_mps_data_model, stating that it is the parsed
MPS/QPS storage being adopted without copying and is left moved-from afterward;
keep the documentation consistent with the corresponding free function.

In `@cpp/src/pdlp/cpu_optimization_problem.cpp`:
- Around line 1139-1183: adopt_from_mps_data_model bypasses validation performed
by the matrix setters, allowing malformed CSR data to propagate. Add lightweight
cuopt_expects invariant checks in this method for A_ and
Q_values_/Q_indices_/Q_offsets_ nullability, offset lengths, and index/value
size consistency, mirroring set_csr_constraint_matrix and
set_quadratic_objective_matrix before downstream use.
- Around line 1114-1135: Replace the positional aggregate initialization in
move_quadratic_constraints_from_model with explicit field-by-field assignments,
mirroring the named-assignment pattern used by add_quadratic_constraint.
Populate each destination quadratic_constraint_t member from qc, preserving move
semantics for strings and vectors, then append the fully initialized object to
converted.

In `@cpp/tests/linear_programming/unit_tests/solution_interface_test.cu`:
- Around line 495-517: Add a test in SolutionInterfaceTest that builds a model
with a non-empty quadratic objective using read_lp_from_string, then passes it
to expect_adopt_matches_populate. Keep the existing quadratic-constraint
coverage and ensure the new case exercises the quadratic objective values,
indices, and offsets comparison through the adopt path.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d3d99f78-2913-436c-9b2b-fed8f8d760fc

📥 Commits

Reviewing files that changed from the base of the PR and between 940ec59 and dab644e.

📒 Files selected for processing (6)

• cpp/cuopt_cli.cpp
• cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp
• cpp/include/cuopt/mathematical_optimization/optimization_problem_utils.hpp
• cpp/src/pdlp/cpu_optimization_problem.cpp
• cpp/src/pdlp/cuopt_c.cpp
• cpp/tests/linear_programming/unit_tests/solution_interface_test.cu

[github-actions]

CI Test Summary

✅ All 31 test job(s) passed.