Fold libmps_parser into libcuopt#1193
Open
mlubin wants to merge 5 commits into
Open
Conversation
mlubin
added
breaking
Contributor
Author
|
/ok to test b71d4ae |
Collaborator
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRemoves standalone mps_parser across build/CI/conda and migrates all C++/Python/tests/docs to cuopt::linear_programming::io parser/writer, integrating io sources into libcuopt, updating solver interfaces, and adjusting dependencies and workflows accordingly. ChangesMPS IO unification and deprecation of standalone mps_parser
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
python/cuopt/cuopt/linear_programming/mps_parser/utilities/__init__.py (1)
4-9:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd a deprecation shim for legacy parser import paths before hard removal.
Line 4 switches consumers to the new module path, but this migration needs a temporary compatibility path (with
DeprecationWarning+ removal version) so existingcuopt_mps_parserimports don’t fail abruptly.Suggested compatibility shim (example)
+ # python/cuopt/cuopt/linear_programming/cuopt_mps_parser/utilities/__init__.py + import warnings + from cuopt.linear_programming.mps_parser.utilities.exception_handler import ( + InputRuntimeError, + InputValidationError, + OutOfMemoryError, + catch_mps_parser_exception, + ) + + warnings.warn( + "cuopt.linear_programming.cuopt_mps_parser.utilities is deprecated; " + "use cuopt.linear_programming.mps_parser.utilities. " + "This path will be removed in a future release.", + DeprecationWarning, + stacklevel=2, + )As per coding guidelines: “For Python/public API breaking changes, require a
DeprecationWarningpath rather than removing immediately.”🤖 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 `@python/cuopt/cuopt/linear_programming/mps_parser/utilities/__init__.py` around lines 4 - 9, Add a deprecation shim module that re-exports the parser exceptions and helper while emitting a DeprecationWarning pointing to the new path and a removal version; specifically, create a tiny compatibility module that imports InputRuntimeError, InputValidationError, OutOfMemoryError, and catch_mps_parser_exception from cuopt.linear_programming.mps_parser.utilities and re-exports them, and call warnings.warn(..., DeprecationWarning, stacklevel=2) with a clear message mentioning the new import path and the planned removal version so existing cuopt_mps_parser imports keep working temporarily.cpp/src/parsers/mps_writer.cpp (2)
56-63:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve CSR offsets when the matrix has zero stored nonzeros.
create_view()currently skipsset_csr_constraint_matrix()whenA_valuesis empty, butwrite()still assumesconstraint_matrix_offsetsis populated for every row. A valid zero-NNZ matrix then drops its row offsets and can hit out-of-bounds reads when serializing columns.Suggested fix
- if (!A_values.empty()) { + if (!A_offsets.empty()) { view.set_csr_constraint_matrix(A_values.data(), static_cast<i_t>(A_values.size()), A_indices.data(), static_cast<i_t>(A_indices.size()), A_offsets.data(), static_cast<i_t>(A_offsets.size())); }🤖 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/parsers/mps_writer.cpp` around lines 56 - 63, create_view() currently skips calling set_csr_constraint_matrix() when A_values is empty, which causes constraint_matrix_offsets to be missing for zero-NNZ matrices and leads to out-of-bounds reads in write(); always register the CSR structure so offsets are preserved even if there are no stored nonzeros by calling set_csr_constraint_matrix() whenever A_offsets (or the number of rows) is present: pass A_offsets.data() and its size, and if A_values/A_indices are empty pass nullptr (or .data() from empty vectors) and zero sizes for the value/index arrays so the offsets are stored; update the call site in create_view() (the block that currently guards set_csr_constraint_matrix()) to unconditionally set the CSR offsets (while handling empty value/index arrays) so write() finds a populated constraint_matrix_offsets.
387-400:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the declared row names in the
RANGESsection.
ROWSandRHShonorproblem_.get_row_names(), butRANGEShard-codesR${i}. For named ranged constraints, the emittedRNG1entries no longer match any declared row, so the written MPS is wrong.Suggested fix
for (size_t i = 0; i < (size_t)n_constraints; i++) { if (constraint_lower_bounds[i] != -std::numeric_limits<f_t>::infinity() && constraint_upper_bounds[i] != std::numeric_limits<f_t>::infinity() && constraint_lower_bounds[i] != constraint_upper_bounds[i]) { if (!has_ranges) { mps_file << "RANGES\n"; has_ranges = true; } - std::string row_name = "R" + std::to_string(i); + std::string row_name = i < problem_.get_row_names().size() + ? problem_.get_row_names()[i] + : "R" + std::to_string(i); mps_file << " RNG1 " << row_name << " " << (constraint_upper_bounds[i] - constraint_lower_bounds[i]) << "\n"; } }🤖 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/parsers/mps_writer.cpp` around lines 387 - 400, The RANGES block currently uses a generated name "R"+i which can mismatch declared rows; change the logic in mps_writer.cpp (the RANGES emission around has_ranges and the RNG1 line) to use the actual row name from problem_.get_row_names() (or the same row name array used by the ROWS and RHS sections) for the variable row_name instead of hard-coding "R"+std::to_string(i), and keep the same range value expression (constraint_upper_bounds[i] - constraint_lower_bounds[i]) and the has_ranges flag behavior.python/cuopt/cuopt/linear_programming/mps_parser/parser.py (1)
12-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate
ParseMpsarguments before entering the Cython/C++ layer.At Line [12], invalid inputs (e.g., non-path
mps_file_path, non-boolfixed_mps_format) will fail later with less actionable errors.Suggested patch
+import os import numpy as np @@ `@catch_mps_parser_exception` def ParseMps(mps_file_path, fixed_mps_format=False): @@ - return parser_wrapper.ParseMps(mps_file_path, fixed_mps_format) + if not isinstance(fixed_mps_format, bool): + raise TypeError("fixed_mps_format must be a bool") + if not isinstance(mps_file_path, (str, os.PathLike)): + raise TypeError("mps_file_path must be a path-like value") + mps_file_path = os.fspath(mps_file_path) + if not mps_file_path: + raise ValueError("mps_file_path cannot be empty") + return parser_wrapper.ParseMps(mps_file_path, fixed_mps_format)As per coding guidelines, "Flag missing input validation at library and server boundaries" and "Error messages that expose internals vs. user-actionable messages."
🤖 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 `@python/cuopt/cuopt/linear_programming/mps_parser/parser.py` around lines 12 - 50, The public ParseMps wrapper does not validate inputs before delegating to parser_wrapper.ParseMps; add pre-call validation in ParseMps to ensure mps_file_path is a str or os.PathLike (or at minimum non-empty string) and fixed_mps_format is a bool, raising clear TypeError/ValueError messages if checks fail, then call parser_wrapper.ParseMps with the validated values; reference the ParseMps function and the parser_wrapper.ParseMps call when locating where to add these checks.cpp/include/cuopt/linear_programming/parsers/writer.hpp (1)
10-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd a direct
#include <string>to keep this header self-contained.Line 26 uses
std::stringin the function signature, but this header does not directly include<string>. Whilestd::stringis available transitively throughdata_model_view.hpp, the IWYU principle requires direct inclusion of what is used to avoid fragility from include-order changes or future dependency modifications.Proposed patch
`#pragma` once `#include` <cuopt/linear_programming/parsers/data_model_view.hpp> +#include <string>🤖 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/linear_programming/parsers/writer.hpp` around lines 10 - 26, The header declares the template function write_mps(...) which uses std::string but does not include <string>, violating IWYU; make this header self-contained by adding a direct `#include` <string> at the top of cpp/include/cuopt/linear_programming/parsers/writer.hpp (above or alongside the existing includes) so the declaration of write_mps(const data_model_view_t<i_t, f_t>&, const std::string&) does not rely on transitive includes.cpp/src/pdlp/utilities/cython_solve.cu (1)
203-237:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard batch inputs before memory-based thread calculation.
Line 232 can divide by zero (
needed_memory), and Line 221 dereferences raw pointers without null checks. Empty batches or malformed entries can crash batch solve.🔧 Suggested fix
static int compute_max_thread( const std::vector<cuopt::linear_programming::parsers::data_model_view_t<int, double>*>& data_models) { constexpr std::size_t max_total = 4; + cuopt_expects(!data_models.empty(), + error_type_t::ValidationError, + "Batch solve requires at least one problem."); // Computing on the total_mem as LP is suppose to run on a single exclusive GPU // On CPU-only hosts cudaMemGetInfo will fail; fall back to single-threaded batch. std::size_t free_mem, total_mem; auto cuda_err = cudaMemGetInfo(&free_mem, &total_mem); @@ // Approximate the necessary memory for each problem std::size_t needed_memory = 0; - for (const auto data_model : data_models) { + for (const auto* data_model : data_models) { + cuopt_expects(data_model != nullptr, + error_type_t::ValidationError, + "Batch solve received a null data model pointer."); const int nb_variables = data_model->get_objective_coefficients().size(); const int nb_constraints = data_model->get_constraint_bounds().size(); @@ 8; } + cuopt_expects(needed_memory > 0, + error_type_t::ValidationError, + "Batch solve cannot estimate memory for empty/invalid models."); - const int res = std::min(max_total, std::min(total_mem / needed_memory, data_models.size())); + const int res = static_cast<int>( + std::min(max_total, std::min(total_mem / needed_memory, data_models.size()))); cuopt_expects( res > 0, error_type_t::RuntimeError, "Problems too big to be solved in batch mode.");As per coding guidelines: "Flag missing input validation at library and server boundaries".
🤖 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/utilities/cython_solve.cu` around lines 203 - 237, compute_max_thread currently dereferences raw pointers from data_models and divides by needed_memory without validating inputs; add upfront guards to (1) return 0 or throw if data_models.empty(), (2) skip or reject null entries in data_models before using data_model->get_objective_coefficients(), get_constraint_bounds(), get_constraint_matrix_values(), get_constraint_matrix_indices(), and get_constraint_matrix_offsets(), and (3) after summing needed_memory, check if needed_memory == 0 and handle it (return 0 or raise a clear error) before computing total_mem / needed_memory and calling std::min; ensure the error path uses cuopt_expects or an appropriate error_type_t and provides a descriptive message.
🤖 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.
Inline comments:
In `@benchmarks/linear_programming/cuopt/initial_problem_check.hpp`:
- Line 33: The header is not self-contained because it uses
cuopt::linear_programming::parsers::mps_data_model_t in the function signature
without including its declaration; fix this by adding an `#include` of the header
that defines cuopt::linear_programming::parsers::mps_data_model_t at the top of
initial_problem_check.hpp (or, if the type is suitable for a forward
declaration, add a proper forward declaration of that template/type in the
cuopt::linear_programming::parsers namespace), ensuring the declaration for
mps_data_model_t is present so the function signature compiles without relying
on transitive includes.
In `@build.sh`:
- Line 66: The BUILD_DIRS assignment contains a stale/undefined variable
CUOPT_SERVICE_CLIENT_BUILD_DIR which results in an empty item and makes the
clean target out of sync; edit the BUILD_DIRS definition to remove
CUOPT_SERVICE_CLIENT_BUILD_DIR or replace it with the correct variable name (if
it was renamed elsewhere) so BUILD_DIRS only contains defined variables like
LIBCUOPT_BUILD_DIR, CUOPT_BUILD_DIR, CUOPT_SERVER_BUILD_DIR,
CUOPT_SH_CLIENT_BUILD_DIR, PY_LIBCUOPT_BUILD_DIR, and DOCS_BUILD_DIR; verify the
clean target uses the updated BUILD_DIRS after the change.
In
`@cpp/include/cuopt/linear_programming/parsers/utilities/cython_mps_parser.hpp`:
- Around line 10-18: The header declares call_parse_mps(const std::string&,
bool) but doesn't include <string>, so make the header self-contained by adding
the missing include: add `#include` <string> at the top of
cuopt::cython::cython_mps_parser.hpp (so call_parse_mps and any other uses of
std::string in this header are valid without relying on transitive includes).
In `@cpp/src/pdlp/cuopt_c.cpp`:
- Line 29: Remove the file-scope using namespace directives and replace them
with explicit qualifications or narrow scoped using-declarations: remove
occurrences of "using namespace cuopt::linear_programming::parsers" and the
other file-scope "using namespace cuopt::linear_programming", then update all
uses of parser/LP symbols (e.g., mps_data_model_t, parse_mps) to fully qualified
names like cuopt::linear_programming::parsers::mps_data_model_t and
cuopt::linear_programming::parsers::parse_mps, or introduce local using
declarations inside functions/blocks for only the few symbols used to avoid
polluting global scope. Ensure every reference to types and functions from
cuopt::linear_programming is explicitly qualified or locally imported.
In `@python/cuopt/cuopt/linear_programming/__init__.py`:
- Line 6: The package root currently only re-exports ParseMps; add a re-export
for toDict so users can access cuopt.linear_programming.toDict as documented.
Import toDict from its defining module (e.g.,
cuopt.linear_programming.mps_parser where ParseMps is imported) and include it
in the module exports (alongside ParseMps and any __all__ list) so the symbol is
available at the package root.
In `@python/cuopt/cuopt/linear_programming/mps_parser/__init__.py`:
- Line 4: Add a deprecation shim module named cuopt_mps_parser that re-exports
the new symbols and emits a DeprecationWarning telling users to migrate to
cuopt.linear_programming.mps_parser; specifically, import ParseMps and toDict
from cuopt.linear_programming.mps_parser, call warnings.warn("cuopt_mps_parser
is deprecated; use cuopt.linear_programming.mps_parser instead and will be
removed in X.Y", DeprecationWarning, stacklevel=2) (replace X.Y with the planned
removal version), and set __all__ = ["ParseMps","toDict"] so existing imports
continue to work while warning users.
---
Outside diff comments:
In `@cpp/include/cuopt/linear_programming/parsers/writer.hpp`:
- Around line 10-26: The header declares the template function write_mps(...)
which uses std::string but does not include <string>, violating IWYU; make this
header self-contained by adding a direct `#include` <string> at the top of
cpp/include/cuopt/linear_programming/parsers/writer.hpp (above or alongside the
existing includes) so the declaration of write_mps(const data_model_view_t<i_t,
f_t>&, const std::string&) does not rely on transitive includes.
In `@cpp/src/parsers/mps_writer.cpp`:
- Around line 56-63: create_view() currently skips calling
set_csr_constraint_matrix() when A_values is empty, which causes
constraint_matrix_offsets to be missing for zero-NNZ matrices and leads to
out-of-bounds reads in write(); always register the CSR structure so offsets are
preserved even if there are no stored nonzeros by calling
set_csr_constraint_matrix() whenever A_offsets (or the number of rows) is
present: pass A_offsets.data() and its size, and if A_values/A_indices are empty
pass nullptr (or .data() from empty vectors) and zero sizes for the value/index
arrays so the offsets are stored; update the call site in create_view() (the
block that currently guards set_csr_constraint_matrix()) to unconditionally set
the CSR offsets (while handling empty value/index arrays) so write() finds a
populated constraint_matrix_offsets.
- Around line 387-400: The RANGES block currently uses a generated name "R"+i
which can mismatch declared rows; change the logic in mps_writer.cpp (the RANGES
emission around has_ranges and the RNG1 line) to use the actual row name from
problem_.get_row_names() (or the same row name array used by the ROWS and RHS
sections) for the variable row_name instead of hard-coding
"R"+std::to_string(i), and keep the same range value expression
(constraint_upper_bounds[i] - constraint_lower_bounds[i]) and the has_ranges
flag behavior.
In `@cpp/src/pdlp/utilities/cython_solve.cu`:
- Around line 203-237: compute_max_thread currently dereferences raw pointers
from data_models and divides by needed_memory without validating inputs; add
upfront guards to (1) return 0 or throw if data_models.empty(), (2) skip or
reject null entries in data_models before using
data_model->get_objective_coefficients(), get_constraint_bounds(),
get_constraint_matrix_values(), get_constraint_matrix_indices(), and
get_constraint_matrix_offsets(), and (3) after summing needed_memory, check if
needed_memory == 0 and handle it (return 0 or raise a clear error) before
computing total_mem / needed_memory and calling std::min; ensure the error path
uses cuopt_expects or an appropriate error_type_t and provides a descriptive
message.
In `@python/cuopt/cuopt/linear_programming/mps_parser/parser.py`:
- Around line 12-50: The public ParseMps wrapper does not validate inputs before
delegating to parser_wrapper.ParseMps; add pre-call validation in ParseMps to
ensure mps_file_path is a str or os.PathLike (or at minimum non-empty string)
and fixed_mps_format is a bool, raising clear TypeError/ValueError messages if
checks fail, then call parser_wrapper.ParseMps with the validated values;
reference the ParseMps function and the parser_wrapper.ParseMps call when
locating where to add these checks.
In `@python/cuopt/cuopt/linear_programming/mps_parser/utilities/__init__.py`:
- Around line 4-9: Add a deprecation shim module that re-exports the parser
exceptions and helper while emitting a DeprecationWarning pointing to the new
path and a removal version; specifically, create a tiny compatibility module
that imports InputRuntimeError, InputValidationError, OutOfMemoryError, and
catch_mps_parser_exception from cuopt.linear_programming.mps_parser.utilities
and re-exports them, and call warnings.warn(..., DeprecationWarning,
stacklevel=2) with a clear message mentioning the new import path and the
planned removal version so existing cuopt_mps_parser imports keep working
temporarily.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 717ef8bd-c8a3-4523-8dbd-484dd68c62e0
📒 Files selected for processing (120)
.github/workflows/build.yaml.github/workflows/pr.yamlbenchmarks/linear_programming/cuopt/benchmark_helper.hppbenchmarks/linear_programming/cuopt/initial_problem_check.hppbenchmarks/linear_programming/cuopt/run_mip.cppbenchmarks/linear_programming/cuopt/run_pdlp.cubuild.shci/build_python.shci/build_wheel_cuopt.shci/build_wheel_cuopt_mps_parser.shci/build_wheel_libcuopt.shci/docker/Dockerfileci/release/update-version.shci/test_self_hosted_service.shci/test_wheel_cuopt.shci/test_wheel_cuopt_server.shconda/recipes/cuopt/recipe.yamlconda/recipes/libcuopt/recipe.yamlconda/recipes/mps-parser/conda_build_config.yamlconda/recipes/mps-parser/recipe.yamlcpp/CMakeLists.txtcpp/cuopt_cli.cppcpp/include/cuopt/linear_programming/optimization_problem_utils.hppcpp/include/cuopt/linear_programming/parsers/data_model_view.hppcpp/include/cuopt/linear_programming/parsers/mps_data_model.hppcpp/include/cuopt/linear_programming/parsers/mps_writer.hppcpp/include/cuopt/linear_programming/parsers/parser.hppcpp/include/cuopt/linear_programming/parsers/utilities/cython_mps_parser.hppcpp/include/cuopt/linear_programming/parsers/writer.hppcpp/include/cuopt/linear_programming/solve.hppcpp/include/cuopt/linear_programming/utilities/cython_solve.hppcpp/libmps_parser/CMakeLists.txtcpp/libmps_parser/cmake/thirdparty/get_gtest.cmakecpp/libmps_parser/src/utilities/cython_mps_parser.cppcpp/libmps_parser/tests/CMakeLists.txtcpp/libmps_parser/tests/utilities/common_utils.hppcpp/src/CMakeLists.txtcpp/src/branch_and_bound/pseudo_costs.cppcpp/src/mip_heuristics/solve.cucpp/src/parsers/CMakeLists.txtcpp/src/parsers/data_model_view.cppcpp/src/parsers/mps_data_model.cppcpp/src/parsers/mps_parser.cppcpp/src/parsers/mps_parser_internal.hppcpp/src/parsers/mps_writer.cppcpp/src/parsers/parser.cppcpp/src/parsers/utilities/cython_mps_parser.cppcpp/src/parsers/utilities/error.hppcpp/src/parsers/writer.cppcpp/src/pdlp/cpu_optimization_problem.cppcpp/src/pdlp/cuopt_c.cppcpp/src/pdlp/optimization_problem.cucpp/src/pdlp/solve.cucpp/src/pdlp/solve.cuhcpp/src/pdlp/utilities/cython_solve.cucpp/tests/CMakeLists.txtcpp/tests/dual_simplex/unit_tests/solve.cppcpp/tests/dual_simplex/unit_tests/solve_barrier.cucpp/tests/linear_programming/CMakeLists.txtcpp/tests/linear_programming/grpc/grpc_integration_test.cppcpp/tests/linear_programming/mps_parser_test.cppcpp/tests/linear_programming/pdlp_test.cucpp/tests/linear_programming/unit_tests/optimization_problem_test.cucpp/tests/linear_programming/unit_tests/presolve_test.cucpp/tests/linear_programming/unit_tests/solution_interface_test.cucpp/tests/linear_programming/utilities/pdlp_test_utilities.cuhcpp/tests/mip/bounds_standardization_test.cucpp/tests/mip/cuts_test.cucpp/tests/mip/determinism_test.cucpp/tests/mip/doc_example_test.cucpp/tests/mip/elim_var_remap_test.cucpp/tests/mip/feasibility_jump_tests.cucpp/tests/mip/incumbent_callback_test.cucpp/tests/mip/integer_with_real_bounds.cucpp/tests/mip/load_balancing_test.cucpp/tests/mip/mip_utils.cuhcpp/tests/mip/miplib_test.cucpp/tests/mip/multi_probe_test.cucpp/tests/mip/presolve_test.cucpp/tests/mip/problem_test.cucpp/tests/mip/semi_continuous_test.cucpp/tests/mip/server_test.cucpp/tests/mip/termination_test.cucpp/tests/mip/unit_test.cucpp/tests/qp/unit_tests/no_constraints.cucpp/tests/qp/unit_tests/two_variable_test.cucpp/tests/utilities/inline_mps_test_utils.hppdependencies.yamlpython/cuopt/CMakeLists.txtpython/cuopt/cuopt/CMakeLists.txtpython/cuopt/cuopt/linear_programming/CMakeLists.txtpython/cuopt/cuopt/linear_programming/LICENSEpython/cuopt/cuopt/linear_programming/README.mdpython/cuopt/cuopt/linear_programming/__init__.pypython/cuopt/cuopt/linear_programming/cuopt_mps_parser/VERSIONpython/cuopt/cuopt/linear_programming/cuopt_mps_parser/__init__.pypython/cuopt/cuopt/linear_programming/cuopt_mps_parser/_version.pypython/cuopt/cuopt/linear_programming/data_model/CMakeLists.txtpython/cuopt/cuopt/linear_programming/data_model/data_model.pxdpython/cuopt/cuopt/linear_programming/mps_parser/CMakeLists.txtpython/cuopt/cuopt/linear_programming/mps_parser/__init__.pypython/cuopt/cuopt/linear_programming/mps_parser/parser.pxdpython/cuopt/cuopt/linear_programming/mps_parser/parser.pypython/cuopt/cuopt/linear_programming/mps_parser/parser_wrapper.pyxpython/cuopt/cuopt/linear_programming/mps_parser/utilities/__init__.pypython/cuopt/cuopt/linear_programming/mps_parser/utilities/exception_handler.pypython/cuopt/cuopt/linear_programming/problem.pypython/cuopt/cuopt/linear_programming/pyproject.tomlpython/cuopt/cuopt/linear_programming/solver/CMakeLists.txtpython/cuopt/cuopt/linear_programming/solver/solver.pypython/cuopt/cuopt/tests/linear_programming/test_cpu_only_execution.pypython/cuopt/cuopt/tests/linear_programming/test_incumbent_callbacks.pypython/cuopt/cuopt/tests/linear_programming/test_lp_solver.pypython/cuopt/cuopt/tests/linear_programming/test_parser.pypython/cuopt/pyproject.tomlpython/cuopt_self_hosted/cuopt_sh_client/cuopt_self_host_client.pypython/cuopt_self_hosted/pyproject.tomlpython/cuopt_server/cuopt_server/tests/test_pdlp_warmstart.pypython/libcuopt/pyproject.tomlregression/benchmark_scripts/utils.py
💤 Files with no reviewable changes (27)
- python/cuopt/cuopt/linear_programming/cuopt_mps_parser/VERSION
- cpp/libmps_parser/cmake/thirdparty/get_gtest.cmake
- ci/test_wheel_cuopt_server.sh
- python/cuopt/cuopt/linear_programming/LICENSE
- ci/build_wheel_libcuopt.sh
- python/cuopt/cuopt/linear_programming/README.md
- conda/recipes/cuopt/recipe.yaml
- python/cuopt/cuopt/linear_programming/cuopt_mps_parser/init.py
- ci/release/update-version.sh
- python/cuopt/CMakeLists.txt
- cpp/libmps_parser/CMakeLists.txt
- python/cuopt/cuopt/linear_programming/cuopt_mps_parser/_version.py
- python/cuopt/pyproject.toml
- ci/build_wheel_cuopt_mps_parser.sh
- ci/docker/Dockerfile
- ci/test_self_hosted_service.sh
- cpp/libmps_parser/tests/CMakeLists.txt
- conda/recipes/mps-parser/conda_build_config.yaml
- conda/recipes/mps-parser/recipe.yaml
- python/libcuopt/pyproject.toml
- python/cuopt/cuopt/linear_programming/CMakeLists.txt
- cpp/libmps_parser/tests/utilities/common_utils.hpp
- python/cuopt_self_hosted/pyproject.toml
- python/cuopt/cuopt/linear_programming/pyproject.toml
- dependencies.yaml
- ci/test_wheel_cuopt.sh
- cpp/libmps_parser/src/utilities/cython_mps_parser.cpp
Contributor
Author
|
/ok to test 4ef676e |
Contributor
Author
|
/ok to test 76257b4 |
chris-maes
reviewed
May 8, 2026
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cpp/include/cuopt/linear_programming/solve.hpp (1)
61-63:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAlign Doxygen parameter descriptions with migrated
io::mps_data_model_tsignaturesThe
@paramdocs still describe legacy/problem-type inputs while the signatures now takecuopt::linear_programming::io::mps_data_model_t(Lines 74, 110, 143). This can mislead callers of this public header.Suggested doc update
- * `@param`[in] mps_data_model An optimization_problem_t<i_t, f_t> object with a - * representation of a linear program + * `@param`[in] mps_data_model An io::mps_data_model_t<i_t, f_t> object with a + * representation of a linear program @@ - * `@param`[in] user_problem A dual_simplex::user_problem_t<i_t, f_t> object with a - * representation of a linear program. + * `@param`[in] mps_data_model An io::mps_data_model_t<i_t, f_t> object with a + * representation of a linear program. @@ - * `@param`[in] mps_data_model An optimization_problem_t<i_t, f_t> object with a - * representation of a linear program + * `@param`[in] mps_data_model An io::mps_data_model_t<i_t, f_t> object with a + * representation of a linear programAs per coding guidelines: “Verify parameter descriptions match actual types/behavior” for
cpp/include/cuopt/**/*.Also applies to: 96-100, 136-138, 74-75, 110-110, 143-143
🤖 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/linear_programming/solve.hpp` around lines 61 - 63, Doxygen `@param` entries for the functions in solve.hpp incorrectly reference the legacy optimization_problem_t<i_t,f_t>; update the parameter descriptions to match the current type cuopt::linear_programming::io::mps_data_model_t and its semantics: change the description for mps_data_model to state it is a cuopt::linear_programming::io::mps_data_model_t representing the LP problem (including any expected ownership/constness and whether it contains constraints, objective, and variable metadata), and ensure handle_ptr doc refers to raft::handle_t with its CUDA stream; apply the same replacement to all affected docblocks (the occurrences around the symbols/params mps_data_model and handle_ptr at the other comment locations).cpp/src/pdlp/utilities/cython_solve.cu (1)
203-233:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard zero-memory/empty-batch path before thread-count calculation
Line 231 computes
total_mem / needed_memorywithout validatingneeded_memory > 0. With an empty batch (or empty data models), this can trigger undefined behavior beforecuopt_expectsruns.Suggested fix
static int compute_max_thread( const std::vector<cuopt::linear_programming::io::data_model_view_t<int, double>*>& data_models) { + if (data_models.empty()) { return 1; } constexpr std::size_t max_total = 4; @@ std::size_t needed_memory = 0; @@ + cuopt_expects( + needed_memory > 0, + error_type_t::ValidationError, + "Batch solve requires non-empty problem models."); + const int res = std::min(max_total, std::min(total_mem / needed_memory, data_models.size())); @@ std::pair<std::vector<std::unique_ptr<solver_ret_t>>, double> call_batch_solve( std::vector<cuopt::linear_programming::io::data_model_view_t<int, double>*> data_models, cuopt::linear_programming::solver_settings_t<int, double>* solver_settings) { @@ const std::size_t size = data_models.size(); + if (size == 0) { return {{}, 0.0}; }Also applies to: 251-269
🤖 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/utilities/cython_solve.cu` around lines 203 - 233, In compute_max_thread, guard the division by needed_memory by checking for an empty batch or zero needed_memory before computing total_mem / needed_memory: if data_models.empty() or needed_memory == 0, return a safe default (e.g., 1) or handle as an empty-batch case instead of performing the division; update the same pattern in the other similar block referenced around lines 251-269 (the equivalent function/logic) so both places validate needed_memory > 0 (and/or data_models not empty) prior to doing total_mem / needed_memory and before calling cuopt_expects.
🤖 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.
Outside diff comments:
In `@cpp/include/cuopt/linear_programming/solve.hpp`:
- Around line 61-63: Doxygen `@param` entries for the functions in solve.hpp
incorrectly reference the legacy optimization_problem_t<i_t,f_t>; update the
parameter descriptions to match the current type
cuopt::linear_programming::io::mps_data_model_t and its semantics: change the
description for mps_data_model to state it is a
cuopt::linear_programming::io::mps_data_model_t representing the LP problem
(including any expected ownership/constness and whether it contains constraints,
objective, and variable metadata), and ensure handle_ptr doc refers to
raft::handle_t with its CUDA stream; apply the same replacement to all affected
docblocks (the occurrences around the symbols/params mps_data_model and
handle_ptr at the other comment locations).
In `@cpp/src/pdlp/utilities/cython_solve.cu`:
- Around line 203-233: In compute_max_thread, guard the division by
needed_memory by checking for an empty batch or zero needed_memory before
computing total_mem / needed_memory: if data_models.empty() or needed_memory ==
0, return a safe default (e.g., 1) or handle as an empty-batch case instead of
performing the division; update the same pattern in the other similar block
referenced around lines 251-269 (the equivalent function/logic) so both places
validate needed_memory > 0 (and/or data_models not empty) prior to doing
total_mem / needed_memory and before calling cuopt_expects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 794733c2-84e9-42d9-86a3-80cc9b4af474
📒 Files selected for processing (68)
benchmarks/linear_programming/cuopt/benchmark_helper.hppbenchmarks/linear_programming/cuopt/initial_problem_check.hppbenchmarks/linear_programming/cuopt/run_mip.cppbenchmarks/linear_programming/cuopt/run_pdlp.cucpp/CMakeLists.txtcpp/cuopt_cli.cppcpp/include/cuopt/linear_programming/io/data_model_view.hppcpp/include/cuopt/linear_programming/io/mps_data_model.hppcpp/include/cuopt/linear_programming/io/mps_writer.hppcpp/include/cuopt/linear_programming/io/parser.hppcpp/include/cuopt/linear_programming/io/utilities/cython_mps_parser.hppcpp/include/cuopt/linear_programming/io/writer.hppcpp/include/cuopt/linear_programming/optimization_problem_utils.hppcpp/include/cuopt/linear_programming/solve.hppcpp/include/cuopt/linear_programming/utilities/cython_solve.hppcpp/src/CMakeLists.txtcpp/src/branch_and_bound/pseudo_costs.cppcpp/src/io/CMakeLists.txtcpp/src/io/data_model_view.cppcpp/src/io/mps_data_model.cppcpp/src/io/mps_parser.cppcpp/src/io/mps_parser_internal.hppcpp/src/io/mps_writer.cppcpp/src/io/parser.cppcpp/src/io/utilities/cython_mps_parser.cppcpp/src/io/utilities/error.hppcpp/src/io/writer.cppcpp/src/mip_heuristics/solve.cucpp/src/pdlp/cpu_optimization_problem.cppcpp/src/pdlp/cuopt_c.cppcpp/src/pdlp/optimization_problem.cucpp/src/pdlp/solve.cucpp/src/pdlp/solve.cuhcpp/src/pdlp/utilities/cython_solve.cucpp/tests/CMakeLists.txtcpp/tests/dual_simplex/unit_tests/solve.cppcpp/tests/dual_simplex/unit_tests/solve_barrier.cucpp/tests/linear_programming/CMakeLists.txtcpp/tests/linear_programming/grpc/grpc_integration_test.cppcpp/tests/linear_programming/mps_parser_test.cppcpp/tests/linear_programming/pdlp_test.cucpp/tests/linear_programming/unit_tests/optimization_problem_test.cucpp/tests/linear_programming/unit_tests/presolve_test.cucpp/tests/linear_programming/unit_tests/solution_interface_test.cucpp/tests/linear_programming/utilities/pdlp_test_utilities.cuhcpp/tests/mip/bounds_standardization_test.cucpp/tests/mip/cuts_test.cucpp/tests/mip/determinism_test.cucpp/tests/mip/doc_example_test.cucpp/tests/mip/elim_var_remap_test.cucpp/tests/mip/feasibility_jump_tests.cucpp/tests/mip/incumbent_callback_test.cucpp/tests/mip/integer_with_real_bounds.cucpp/tests/mip/load_balancing_test.cucpp/tests/mip/mip_utils.cuhcpp/tests/mip/miplib_test.cucpp/tests/mip/multi_probe_test.cucpp/tests/mip/presolve_test.cucpp/tests/mip/problem_test.cucpp/tests/mip/semi_continuous_test.cucpp/tests/mip/server_test.cucpp/tests/mip/termination_test.cucpp/tests/mip/unit_test.cucpp/tests/qp/unit_tests/no_constraints.cucpp/tests/qp/unit_tests/two_variable_test.cucpp/tests/utilities/inline_mps_test_utils.hpppython/cuopt/cuopt/linear_programming/data_model/data_model.pxdpython/cuopt/cuopt/linear_programming/mps_parser/parser.pxd
✅ Files skipped from review due to trivial changes (9)
- cpp/tests/mip/problem_test.cu
- cpp/src/io/utilities/cython_mps_parser.cpp
- cpp/src/CMakeLists.txt
- cpp/tests/dual_simplex/unit_tests/solve.cpp
- cpp/tests/mip/semi_continuous_test.cu
- cpp/tests/qp/unit_tests/two_variable_test.cu
- cpp/tests/mip/integer_with_real_bounds.cu
- cpp/tests/mip/termination_test.cu
- cpp/tests/mip/incumbent_callback_test.cu
🚧 Files skipped from review as they are similar to previous changes (16)
- benchmarks/linear_programming/cuopt/run_mip.cpp
- cpp/cuopt_cli.cpp
- cpp/src/pdlp/solve.cuh
- benchmarks/linear_programming/cuopt/run_pdlp.cu
- cpp/src/pdlp/cuopt_c.cpp
- cpp/tests/mip/load_balancing_test.cu
- cpp/tests/mip/mip_utils.cuh
- cpp/tests/linear_programming/utilities/pdlp_test_utilities.cuh
- cpp/tests/linear_programming/grpc/grpc_integration_test.cpp
- cpp/tests/CMakeLists.txt
- cpp/src/pdlp/cpu_optimization_problem.cpp
- cpp/tests/mip/feasibility_jump_tests.cu
- cpp/tests/linear_programming/unit_tests/solution_interface_test.cu
- cpp/tests/mip/elim_var_remap_test.cu
- cpp/tests/mip/presolve_test.cu
- cpp/tests/linear_programming/CMakeLists.txt
Merge the standalone libmps_parser C++ library and the cuopt-mps-parser
Python wheel into the main cuopt build, eliminating the separate
distribution while keeping all functionality. The parser sources now live
under cpp/src/parsers/ and the public headers under
cpp/include/cuopt/linear_programming/parsers/, matching the new namespace
cuopt::linear_programming::parsers.
C++:
- Move cpp/libmps_parser/{src,include,tests}/ into the cuopt tree.
- Drop the separate libmps_parser.so target; parser sources compile
into libcuopt.so via cpp/src/parsers/CMakeLists.txt.
- Hoist BZip2/ZLIB find_package + CUOPT_PARSER_WITH_{BZIP2,ZLIB} options
to the top-level cpp build.
- Rename namespace cuopt::mps_parser -> cuopt::linear_programming::parsers
across all sources, headers, tests, and benchmarks.
- Wire the gtest suite (mps_parser_test.cpp) into
cpp/tests/linear_programming/CMakeLists.txt.
Python:
- Move python/cuopt/cuopt/linear_programming/cuopt_mps_parser/ into
python/cuopt/cuopt/linear_programming/mps_parser/, importable as
cuopt.linear_programming.mps_parser (and re-exported as ParseMps/toDict
at the linear_programming package level).
- Fold data_model/ cython binding into the main cuopt wheel; relink
data_model and solver cython modules to cuopt::cuopt.
- Update cython .pxd extern paths and namespace strings to
cuopt/linear_programming/parsers/ and cuopt::linear_programming::parsers.
- Delete the standalone parser-wheel scaffolding (pyproject.toml,
CMakeLists.txt, README, LICENSE, _version.py).
- Update test imports across test_parser.py, test_lp_solver.py,
test_incumbent_callbacks.py, test_cpu_only_execution.py,
test_pdlp_warmstart.py, plus cuopt_self_host_client.py,
problem.py, and regression/benchmark_scripts/utils.py.
Build / packaging / CI:
- Drop the libmps_parser target and the FIND_MPS_PARSER_CPP flag from
build.sh; help text and default action updated accordingly.
- Delete conda/recipes/mps-parser/ and the libmps-parser output (plus
all pin_subpackage references) in conda/recipes/libcuopt/recipe.yaml.
- Drop cuopt-mps-parser =${version} from conda/recipes/cuopt/recipe.yaml.
- Remove the wheel-build-cuopt-mps-parser job, mps_parser_filter, and
ci/build_wheel_cuopt_mps_parser.sh from CI; clean up cuopt_mps_parser
wheelhouse downloads/installs from the remaining ci/*.sh scripts.
- Drop cuopt-mps-parser dependency entries from python/{cuopt,libcuopt,
cuopt_self_hosted}/pyproject.toml and dependencies.yaml; remove the
py_*_cuopt_mps_parser pyproject groups and depends_on_mps_parser.
- Update Dockerfile and ci/release/update-version.sh package lists.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Miles Lubin <mlubin@nvidia.com>
Update docs, READMEs, contributor guidance, agent skills, and license
attribution to reflect the folded layout: parser sources under
cpp/src/parsers/ inside libcuopt.so, and Python parser under
cuopt.linear_programming.mps_parser inside the cuopt wheel.
- docs/cuopt/source/hidden/mps-{api,example}.rst: switch the autofunction
target and example to cuopt.linear_programming.mps_parser.
- docs/cuopt/source/cuopt-server/examples/lp/examples/mps_datamodel_example.py:
update the import + ParseMps call.
- cpp/README.md: rewrite the MPS parser section now that it's compiled
into libcuopt.
- CONTRIBUTING.md: drop libmps_parser / cuopt_mps_parser from the
build.sh examples.
- cpp/include/cuopt/linear_programming/parsers/writer.hpp: drop the
obsolete TODO referencing the standalone libmps_parser.
- skills/cuopt-developer/{SKILL.md,resources/python_bindings.md}: refresh
example build command and CMakeLists snippet.
- thirdparty/THIRD_PARTY_LICENSES: bzip2 and zlib usage attribution moves
from libmps_parser to cuopt.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Miles Lubin <mlubin@nvidia.com>
The directory holds both read and write paths for the MPS/QPS format (parser + writer + data model), so "io" describes it more accurately than "parsers". Per PR review feedback. - cpp/include/cuopt/linear_programming/parsers/ -> .../io/ - cpp/src/parsers/ -> cpp/src/io/ - Namespace cuopt::linear_programming::parsers -> cuopt::linear_programming::io across all sources, headers, tests, benchmarks, and the cython .pxd extern declarations. - CMake: add_subdirectory(parsers) and src/parsers include paths updated to io. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Miles Lubin <mlubin@nvidia.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cpp/tests/linear_programming/mps_parser_test.cpp (1)
27-27: ⚡ Quick winMove test-local helpers to an unnamed namespace.
At Line 27, this file enters
cuopt::linear_programming::ioand defines helper functions with external linkage. Please wrap test-only helpers (read_from_mps,file_exists,compare_data_models) innamespace { ... }(optionally nested inside thecuopt::linear_programming::ioblock) to avoid symbol leakage and potential ODR collisions.As per coding guidelines, "
**/*.{hpp,cpp,cuh,cu,h}: ... use unnamed namespaces for internal linkage."Also applies to: 1401-1401
🤖 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/mps_parser_test.cpp` at line 27, The helper functions read_from_mps, file_exists, and compare_data_models are currently defined with external linkage inside namespace cuopt::linear_programming::io; wrap their definitions in an unnamed namespace (namespace { ... }) so they get internal linkage (you can place the unnamed namespace inside the existing cuopt::linear_programming::io block or around the helper definitions) to prevent symbol leakage and ODR collisions, updating any references in the test file accordingly.
🤖 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/tests/linear_programming/mps_parser_test.cpp`:
- Line 27: The helper functions read_from_mps, file_exists, and
compare_data_models are currently defined with external linkage inside namespace
cuopt::linear_programming::io; wrap their definitions in an unnamed namespace
(namespace { ... }) so they get internal linkage (you can place the unnamed
namespace inside the existing cuopt::linear_programming::io block or around the
helper definitions) to prevent symbol leakage and ODR collisions, updating any
references in the test file accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5f560a1b-8531-4e4e-9ae9-a7c0d46e9975
⛔ Files ignored due to path filters (1)
thirdparty/THIRD_PARTY_LICENSESis excluded by!thirdparty/**
📒 Files selected for processing (127)
.github/workflows/build.yaml.github/workflows/pr.yamlCONTRIBUTING.mdbenchmarks/linear_programming/cuopt/benchmark_helper.hppbenchmarks/linear_programming/cuopt/initial_problem_check.hppbenchmarks/linear_programming/cuopt/run_mip.cppbenchmarks/linear_programming/cuopt/run_pdlp.cubuild.shci/build_python.shci/build_wheel_cuopt.shci/build_wheel_cuopt_mps_parser.shci/build_wheel_libcuopt.shci/docker/Dockerfileci/release/update-version.shci/test_self_hosted_service.shci/test_wheel_cuopt.shci/test_wheel_cuopt_server.shconda/recipes/cuopt/recipe.yamlconda/recipes/libcuopt/recipe.yamlconda/recipes/mps-parser/conda_build_config.yamlconda/recipes/mps-parser/recipe.yamlcpp/CMakeLists.txtcpp/README.mdcpp/cuopt_cli.cppcpp/include/cuopt/linear_programming/io/data_model_view.hppcpp/include/cuopt/linear_programming/io/mps_data_model.hppcpp/include/cuopt/linear_programming/io/mps_writer.hppcpp/include/cuopt/linear_programming/io/parser.hppcpp/include/cuopt/linear_programming/io/utilities/cython_mps_parser.hppcpp/include/cuopt/linear_programming/io/writer.hppcpp/include/cuopt/linear_programming/optimization_problem_utils.hppcpp/include/cuopt/linear_programming/solve.hppcpp/include/cuopt/linear_programming/utilities/cython_solve.hppcpp/libmps_parser/CMakeLists.txtcpp/libmps_parser/cmake/thirdparty/get_gtest.cmakecpp/libmps_parser/src/utilities/cython_mps_parser.cppcpp/libmps_parser/tests/CMakeLists.txtcpp/libmps_parser/tests/utilities/common_utils.hppcpp/src/CMakeLists.txtcpp/src/branch_and_bound/pseudo_costs.cppcpp/src/io/CMakeLists.txtcpp/src/io/data_model_view.cppcpp/src/io/mps_data_model.cppcpp/src/io/mps_parser.cppcpp/src/io/mps_parser_internal.hppcpp/src/io/mps_writer.cppcpp/src/io/parser.cppcpp/src/io/utilities/cython_mps_parser.cppcpp/src/io/utilities/error.hppcpp/src/io/writer.cppcpp/src/mip_heuristics/solve.cucpp/src/pdlp/cpu_optimization_problem.cppcpp/src/pdlp/cuopt_c.cppcpp/src/pdlp/optimization_problem.cucpp/src/pdlp/solve.cucpp/src/pdlp/solve.cuhcpp/src/pdlp/utilities/cython_solve.cucpp/tests/CMakeLists.txtcpp/tests/dual_simplex/unit_tests/solve.cppcpp/tests/dual_simplex/unit_tests/solve_barrier.cucpp/tests/linear_programming/CMakeLists.txtcpp/tests/linear_programming/grpc/grpc_integration_test.cppcpp/tests/linear_programming/mps_parser_test.cppcpp/tests/linear_programming/pdlp_test.cucpp/tests/linear_programming/unit_tests/optimization_problem_test.cucpp/tests/linear_programming/unit_tests/presolve_test.cucpp/tests/linear_programming/unit_tests/solution_interface_test.cucpp/tests/linear_programming/utilities/pdlp_test_utilities.cuhcpp/tests/mip/bounds_standardization_test.cucpp/tests/mip/cuts_test.cucpp/tests/mip/determinism_test.cucpp/tests/mip/doc_example_test.cucpp/tests/mip/elim_var_remap_test.cucpp/tests/mip/feasibility_jump_tests.cucpp/tests/mip/incumbent_callback_test.cucpp/tests/mip/integer_with_real_bounds.cucpp/tests/mip/load_balancing_test.cucpp/tests/mip/mip_utils.cuhcpp/tests/mip/miplib_test.cucpp/tests/mip/multi_probe_test.cucpp/tests/mip/presolve_test.cucpp/tests/mip/problem_test.cucpp/tests/mip/semi_continuous_test.cucpp/tests/mip/server_test.cucpp/tests/mip/termination_test.cucpp/tests/mip/unit_test.cucpp/tests/qp/unit_tests/no_constraints.cucpp/tests/qp/unit_tests/two_variable_test.cucpp/tests/utilities/inline_mps_test_utils.hppdependencies.yamldocs/cuopt/source/cuopt-server/examples/lp/examples/mps_datamodel_example.pydocs/cuopt/source/hidden/mps-api.rstdocs/cuopt/source/hidden/mps-example.rstpython/cuopt/CMakeLists.txtpython/cuopt/cuopt/CMakeLists.txtpython/cuopt/cuopt/linear_programming/CMakeLists.txtpython/cuopt/cuopt/linear_programming/LICENSEpython/cuopt/cuopt/linear_programming/README.mdpython/cuopt/cuopt/linear_programming/__init__.pypython/cuopt/cuopt/linear_programming/cuopt_mps_parser/VERSIONpython/cuopt/cuopt/linear_programming/cuopt_mps_parser/__init__.pypython/cuopt/cuopt/linear_programming/cuopt_mps_parser/_version.pypython/cuopt/cuopt/linear_programming/data_model/CMakeLists.txtpython/cuopt/cuopt/linear_programming/data_model/data_model.pxdpython/cuopt/cuopt/linear_programming/mps_parser/CMakeLists.txtpython/cuopt/cuopt/linear_programming/mps_parser/__init__.pypython/cuopt/cuopt/linear_programming/mps_parser/parser.pxdpython/cuopt/cuopt/linear_programming/mps_parser/parser.pypython/cuopt/cuopt/linear_programming/mps_parser/parser_wrapper.pyxpython/cuopt/cuopt/linear_programming/mps_parser/utilities/__init__.pypython/cuopt/cuopt/linear_programming/mps_parser/utilities/exception_handler.pypython/cuopt/cuopt/linear_programming/problem.pypython/cuopt/cuopt/linear_programming/pyproject.tomlpython/cuopt/cuopt/linear_programming/solver/CMakeLists.txtpython/cuopt/cuopt/linear_programming/solver/solver.pypython/cuopt/cuopt/tests/linear_programming/test_cpu_only_execution.pypython/cuopt/cuopt/tests/linear_programming/test_incumbent_callbacks.pypython/cuopt/cuopt/tests/linear_programming/test_lp_solver.pypython/cuopt/cuopt/tests/linear_programming/test_parser.pypython/cuopt/pyproject.tomlpython/cuopt_self_hosted/cuopt_sh_client/cuopt_self_host_client.pypython/cuopt_self_hosted/pyproject.tomlpython/cuopt_server/cuopt_server/tests/test_pdlp_warmstart.pypython/libcuopt/pyproject.tomlregression/benchmark_scripts/utils.pyskills/cuopt-developer/resources/build_and_test.mdskills/cuopt-developer/resources/python_bindings.md
💤 Files with no reviewable changes (28)
- python/cuopt/cuopt/linear_programming/LICENSE
- cpp/libmps_parser/src/utilities/cython_mps_parser.cpp
- cpp/libmps_parser/cmake/thirdparty/get_gtest.cmake
- python/cuopt/CMakeLists.txt
- python/cuopt_self_hosted/pyproject.toml
- python/cuopt/cuopt/linear_programming/cuopt_mps_parser/VERSION
- python/cuopt/pyproject.toml
- python/cuopt/cuopt/linear_programming/cuopt_mps_parser/init.py
- ci/test_wheel_cuopt_server.sh
- python/cuopt/cuopt/linear_programming/README.md
- conda/recipes/mps-parser/recipe.yaml
- ci/release/update-version.sh
- conda/recipes/mps-parser/conda_build_config.yaml
- cpp/README.md
- python/cuopt/cuopt/linear_programming/cuopt_mps_parser/_version.py
- ci/build_wheel_cuopt_mps_parser.sh
- ci/build_wheel_libcuopt.sh
- ci/test_self_hosted_service.sh
- ci/docker/Dockerfile
- conda/recipes/cuopt/recipe.yaml
- ci/test_wheel_cuopt.sh
- python/cuopt/cuopt/linear_programming/CMakeLists.txt
- cpp/libmps_parser/tests/CMakeLists.txt
- python/libcuopt/pyproject.toml
- python/cuopt/cuopt/linear_programming/pyproject.toml
- cpp/libmps_parser/CMakeLists.txt
- cpp/libmps_parser/tests/utilities/common_utils.hpp
- dependencies.yaml
✅ Files skipped from review due to trivial changes (24)
- docs/cuopt/source/hidden/mps-api.rst
- docs/cuopt/source/hidden/mps-example.rst
- cpp/tests/dual_simplex/unit_tests/solve_barrier.cu
- cpp/tests/mip/semi_continuous_test.cu
- cpp/tests/mip/feasibility_jump_tests.cu
- cpp/src/CMakeLists.txt
- cpp/src/io/utilities/cython_mps_parser.cpp
- python/cuopt/cuopt/linear_programming/mps_parser/init.py
- skills/cuopt-developer/resources/python_bindings.md
- cpp/tests/mip/problem_test.cu
- cpp/tests/mip/termination_test.cu
- cpp/src/io/utilities/error.hpp
- python/cuopt/cuopt/linear_programming/data_model/data_model.pxd
- cpp/include/cuopt/linear_programming/io/parser.hpp
- cpp/tests/qp/unit_tests/no_constraints.cu
- python/cuopt/cuopt/tests/linear_programming/test_incumbent_callbacks.py
- python/cuopt/cuopt/linear_programming/solver/solver.py
- python/cuopt/cuopt/tests/linear_programming/test_parser.py
- cpp/src/io/CMakeLists.txt
- python/cuopt_self_hosted/cuopt_sh_client/cuopt_self_host_client.py
- python/cuopt/cuopt/linear_programming/init.py
- python/cuopt/cuopt/CMakeLists.txt
- cpp/tests/qp/unit_tests/two_variable_test.cu
- cpp/tests/mip/integer_with_real_bounds.cu
🚧 Files skipped from review as they are similar to previous changes (61)
- cpp/include/cuopt/linear_programming/io/mps_writer.hpp
- cpp/src/io/mps_parser_internal.hpp
- python/cuopt/cuopt/linear_programming/data_model/CMakeLists.txt
- benchmarks/linear_programming/cuopt/initial_problem_check.hpp
- python/cuopt_server/cuopt_server/tests/test_pdlp_warmstart.py
- cpp/tests/mip/determinism_test.cu
- cpp/tests/dual_simplex/unit_tests/solve.cpp
- cpp/tests/mip/incumbent_callback_test.cu
- cpp/src/branch_and_bound/pseudo_costs.cpp
- cpp/tests/utilities/inline_mps_test_utils.hpp
- cpp/tests/linear_programming/unit_tests/optimization_problem_test.cu
- benchmarks/linear_programming/cuopt/run_pdlp.cu
- cpp/src/io/writer.cpp
- benchmarks/linear_programming/cuopt/benchmark_helper.hpp
- python/cuopt/cuopt/linear_programming/mps_parser/utilities/init.py
- cpp/tests/mip/bounds_standardization_test.cu
- cpp/include/cuopt/linear_programming/io/data_model_view.hpp
- docs/cuopt/source/cuopt-server/examples/lp/examples/mps_datamodel_example.py
- python/cuopt/cuopt/linear_programming/solver/CMakeLists.txt
- cpp/tests/linear_programming/unit_tests/solution_interface_test.cu
- python/cuopt/cuopt/linear_programming/mps_parser/parser.py
- cpp/include/cuopt/linear_programming/io/utilities/cython_mps_parser.hpp
- cpp/src/mip_heuristics/solve.cu
- cpp/src/pdlp/optimization_problem.cu
- cpp/include/cuopt/linear_programming/optimization_problem_utils.hpp
- cpp/tests/linear_programming/CMakeLists.txt
- cpp/src/pdlp/cpu_optimization_problem.cpp
- cpp/src/io/parser.cpp
- cpp/src/io/mps_writer.cpp
- cpp/tests/CMakeLists.txt
- cpp/src/io/mps_parser.cpp
- cpp/include/cuopt/linear_programming/utilities/cython_solve.hpp
- CONTRIBUTING.md
- python/cuopt/cuopt/linear_programming/mps_parser/parser_wrapper.pyx
- ci/build_wheel_cuopt.sh
- cpp/tests/mip/mip_utils.cuh
- python/cuopt/cuopt/tests/linear_programming/test_cpu_only_execution.py
- cpp/src/io/data_model_view.cpp
- python/cuopt/cuopt/tests/linear_programming/test_lp_solver.py
- cpp/tests/linear_programming/unit_tests/presolve_test.cu
- cpp/tests/mip/load_balancing_test.cu
- conda/recipes/libcuopt/recipe.yaml
- cpp/tests/linear_programming/utilities/pdlp_test_utilities.cuh
- cpp/tests/mip/cuts_test.cu
- cpp/src/pdlp/cuopt_c.cpp
- cpp/include/cuopt/linear_programming/solve.hpp
- cpp/tests/linear_programming/grpc/grpc_integration_test.cpp
- cpp/tests/mip/doc_example_test.cu
- cpp/tests/mip/miplib_test.cu
- cpp/src/pdlp/solve.cu
- python/cuopt/cuopt/linear_programming/problem.py
- python/cuopt/cuopt/linear_programming/mps_parser/CMakeLists.txt
- cpp/include/cuopt/linear_programming/io/writer.hpp
- cpp/CMakeLists.txt
- regression/benchmark_scripts/utils.py
- cpp/tests/mip/unit_test.cu
- cpp/src/pdlp/utilities/cython_solve.cu
- cpp/src/io/mps_data_model.cpp
- build.sh
- .github/workflows/pr.yaml
- ci/build_python.sh
Contributor
Author
|
/ok to test c2458da |
After folding the cuopt-mps-parser wheel into cuopt, the top-level `data_model` Python module no longer exists. Update the sh-cli-api.rst autoclass directive to point at the new path `cuopt.linear_programming.data_model.DataModel`. This unblocks the docs-build CI job, which treats sphinx autodoc import failures as errors. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Miles Lubin <mlubin@nvidia.com>
Contributor
Author
|
/ok to test 777ccdd |
The nitpick_ignore entry suppressing the auto-generated xref for the inherited set_data_model_view method was keyed on the top-level `data_model.DataModel` path that the now-removed cuopt-mps-parser wheel used. After folding the parser into cuopt, the autoclass directive in sh-cli-api.rst points at `cuopt.linear_programming.data_model.DataModel`, so update the ignore entry's prefix to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Miles Lubin <mlubin@nvidia.com>
Contributor
Author
|
/ok to test dd4abd5 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Merge the standalone libmps_parser C++ library and the cuopt-mps-parser Python wheel into the main cuopt build, eliminating the separate distribution while keeping all functionality. The parser sources now live under cpp/src/io/ and the public headers under cpp/include/cuopt/linear_programming/io/, matching the new namespace cuopt::linear_programming::io.
C++:
Python:
Build / packaging / CI:
Migration instructions
MPS/QPS parsing functionality is now bundled directly into the
cuoptPython package. The standalonecuopt-mps-parserpackage (and itscuopt_mps_parseranddata_modelimport names) are no longer published.If you depend on the parser, you need to update both your environment and your imports.
Environment
Conda users. Remove
cuopt-mps-parserfrom your environment files;cuoptnow bundles the parser:pip users. Remove
cuopt-mps-parserfrom yourrequirements.txt/pyproject.tomldependencies.cuopt(andlibcuopt,cuopt-server,cuopt-sh-client) no longer require it transitively:cuopt-cu13==26.6.* -cuopt-mps-parser==26.6.*Run
pip uninstall cuopt-mps-parserin existing environments to clear the old install.Code
The functions and classes are unchanged — only the import paths moved. Update as follows:
import cuopt_mps_parserfrom cuopt.linear_programming import mps_parsercuopt_mps_parser.ParseMps(...)mps_parser.ParseMps(...)cuopt_mps_parser.toDict(...)from cuopt.linear_programming.mps_parser import toDictthentoDict(...)cuopt_mps_parser.parser_wrapper.DataModelmps_parser.parser_wrapper.DataModelfrom cuopt_mps_parser.utilities import InputValidationErrorfrom cuopt.linear_programming.mps_parser.utilities import InputValidationErrorfrom data_model import DataModelfrom cuopt.linear_programming.data_model import DataModelFor convenience,
ParseMpsis also re-exported at the package level: