Conversation
ramakrishnap-nv
left a comment
There was a problem hiding this comment.
Nice fix — the GPU-resident-to-MPS round trip is a reasonable way to reuse the existing distributed ctor. A few things before merging: no tests exercise this new branch on the 2-GPU runner, and the condition doesn't check use_distributed_pdlp independently like the MPS overload does.
| // TODO: handle problems that don't fit on a single GPU by not loading problem in memory at the beginning. | ||
| if (!is_batch_mode && | ||
| settings.method == method_t::PDLP && | ||
| (settings.num_gpus == -1 || settings.num_gpus > 1)) { |
There was a problem hiding this comment.
This only checks method == PDLP && (num_gpus == -1 || num_gpus > 1), but solve_lp_distributed_from_mps also honors an explicit use_distributed_pdlp = true independent of num_gpus. A caller who sets use_distributed_pdlp = true with num_gpus == 1 (allowed downstream, since the only check there is num_gpus >= 1) will silently fall through to single-GPU here instead of distributing or erroring. Might be worth mirroring the if (use_distributed_pdlp) ... else if (...) structure from the MPS overload.
| if (!is_batch_mode && | ||
| settings.method == method_t::PDLP && | ||
| (settings.num_gpus == -1 || settings.num_gpus > 1)) { | ||
| auto mps = op_problem_to_mps_data_model(*gpu_prob); |
There was a problem hiding this comment.
Indentation of this block doesn't match the surrounding style (looks like it wasn't run through clang-format).
| cuopt_expects(gpu_prob != nullptr, | ||
| error_type_t::ValidationError, | ||
| "problem_interface must be either a CPU or GPU optimization problem"); | ||
| // Handle multi-GPU problems |
There was a problem hiding this comment.
Could this get a gtest exercising this specific branch (GPU-resident optimization_problem_t path, not the existing MPS-file-based pdlp_distributed_test.cu coverage) on the 2-GPU runner? Right now this conversion path is untested.
Solving via DataModel/Solve builds the problem directly on the GPU; without the dispatch fix in NVIDIA#1958, use_distributed_pdlp and distributed_pdlp_partitioner are stored but have no effect there (MPS file based solves are unaffected). Document this until NVIDIA#1958 lands. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
/ok to test 9755ee9 |
|
/ok to test 1326b08 |
CI Test Summary2 failed · 30 passed · 0 skipped
|
| error_type_t::ValidationError, | ||
| "problem_interface must be either a CPU or GPU optimization problem"); | ||
| // Handle multi-GPU problems | ||
| // TODO: handle problems that don't fit on a single GPU by not loading problem in memory at the |
There was a problem hiding this comment.
So this makes the C API work? But only for problems that fit into memory on a single GPU?
Description
Issue
Checklist