Skip to content

remove extra or unnecessary parameters in Python gRPC api #1654

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.

Sign up for GitHub

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

Merged
rapids-bot merged 3 commits into from
Aug 3, 2026
Merged

remove extra or unnecessary parameters in Python gRPC api #1654

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cff782f
feat(grpc): add get_result for Python client; require settings for in…
tmckayus Aug 2, 2026
dc593dc
test(grpc): cover get_result for LP/MIP unary and chunked paths
tmckayus Aug 2, 2026
da6dd3e
restore internal plubming for client callback= on incumbents
tmckayus Aug 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cpp/include/cuopt/grpc/cython_grpc_client.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,10 @@ class grpc_python_client_t {
bool delete_job(const std::string& job_id, std::string& error_out);

/**
* @param is_mip When true, fetch a MIP result; otherwise LP.
* Fetch the solution for a completed job. LP vs MIP is determined from the
* server response via grpc_client_t::get_result().
*/
grpc_result_outcome_t result(const std::string& job_id, bool is_mip);
grpc_result_outcome_t result(const std::string& job_id);

/**
* @brief Block until the job completes, collecting all solver log lines.
Expand Down
27 changes: 10 additions & 17 deletions cpp/src/grpc/client/cython_grpc_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ bool grpc_python_client_t::delete_job(const std::string& job_id, std::string& er
return true;
}

grpc_result_outcome_t grpc_python_client_t::result(const std::string& job_id, bool is_mip)
grpc_result_outcome_t grpc_python_client_t::result(const std::string& job_id)
{
grpc_result_outcome_t out;

Expand All @@ -222,26 +222,19 @@ grpc_result_outcome_t grpc_python_client_t::result(const std::string& job_id, bo
return out;
}

out.solution = std::make_unique<solver_ret_t>();
auto remote = impl_->client.get_result<int, double>(job_id);
if (!remote.success) {
out.error_message = remote.error_message;
return out;
}

if (is_mip) {
auto remote = impl_->client.get_mip_result<int, double>(job_id);
if (!remote.success) {
out.error_message = remote.error_message;
out.solution.reset();
return out;
}
out.solution = std::make_unique<solver_ret_t>();
if (remote.is_mip) {
out.solution->problem_type = cuopt::mathematical_optimization::problem_category_t::MIP;
out.solution->mip_ret = remote.solution->to_cpu_mip_ret_t();
out.solution->mip_ret = remote.mip_solution->to_cpu_mip_ret_t();
} else {
auto remote = impl_->client.get_lp_result<int, double>(job_id);
if (!remote.success) {
out.error_message = remote.error_message;
out.solution.reset();
return out;
}
out.solution->problem_type = cuopt::mathematical_optimization::problem_category_t::LP;
out.solution->lp_ret = remote.solution->to_cpu_linear_programming_ret_t();
out.solution->lp_ret = remote.lp_solution->to_cpu_linear_programming_ret_t();
}

out.success = true;
Expand Down
54 changes: 54 additions & 0 deletions cpp/src/grpc/client/grpc_client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -761,6 +761,58 @@ remote_mip_result_t<i_t, f_t> grpc_client_t::get_mip_result(const std::string& j
return result;
}

template <typename i_t, typename f_t>
remote_result_t<i_t, f_t> grpc_client_t::get_result(const std::string& job_id)
{
remote_result_t<i_t, f_t> result;

if (!is_connected()) {
result.error_message = "Not connected to server";
return result;
}

downloaded_result_t dl;
if (!get_result_or_download(job_id, dl)) {
result.error_message = last_error_;
return result;
}

const bool is_mip = dl.was_chunked ? (dl.chunked_header->problem_category() == cuopt::remote::MIP)
: dl.response->has_mip_solution();

if (is_mip) {
if (dl.was_chunked) {
result.mip_solution = std::make_unique<cpu_mip_solution_t<i_t, f_t>>(
chunked_result_to_mip_solution<i_t, f_t>(*dl.chunked_header, dl.chunked_arrays));
} else {
if (!dl.response->has_mip_solution()) {
result.error_message = "GetResult succeeded but no MIP solution in response";
return result;
}
result.mip_solution = std::make_unique<cpu_mip_solution_t<i_t, f_t>>(
map_proto_to_mip_solution<i_t, f_t>(dl.response->mip_solution()));
}
result.is_mip = true;
result.success = true;
return result;
}

if (dl.was_chunked) {
result.lp_solution = std::make_unique<cpu_lp_solution_t<i_t, f_t>>(
chunked_result_to_lp_solution<i_t, f_t>(*dl.chunked_header, dl.chunked_arrays));
} else {
if (!dl.response->has_lp_solution()) {
result.error_message = "GetResult succeeded but no LP solution in response";
return result;
}
result.lp_solution = std::make_unique<cpu_lp_solution_t<i_t, f_t>>(
map_proto_to_lp_solution<i_t, f_t>(dl.response->lp_solution()));
}
result.is_mip = false;
result.success = true;
return result;
}

// =============================================================================
// Polling helper
// =============================================================================
Expand Down Expand Up @@ -1258,6 +1310,7 @@ template submit_result_t grpc_client_t::submit_mip(
template remote_lp_result_t<int32_t, float> grpc_client_t::get_lp_result(const std::string& job_id);
template remote_mip_result_t<int32_t, float> grpc_client_t::get_mip_result(
const std::string& job_id);
template remote_result_t<int32_t, float> grpc_client_t::get_result(const std::string& job_id);
template bool grpc_client_t::upload_chunked_arrays(
const cpu_optimization_problem_t<int32_t, float>& problem,
const cuopt::remote::ChunkedProblemHeader& header,
Expand All @@ -1283,6 +1336,7 @@ template remote_lp_result_t<int32_t, double> grpc_client_t::get_lp_result(
const std::string& job_id);
template remote_mip_result_t<int32_t, double> grpc_client_t::get_mip_result(
const std::string& job_id);
template remote_result_t<int32_t, double> grpc_client_t::get_result(const std::string& job_id);
template bool grpc_client_t::upload_chunked_arrays(
const cpu_optimization_problem_t<int32_t, double>& problem,
const cuopt::remote::ChunkedProblemHeader& header,
Expand Down
22 changes: 22 additions & 0 deletions cpp/src/grpc/client/grpc_client.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,18 @@ struct remote_mip_result_t {
std::unique_ptr<cpu_mip_solution_t<i_t, f_t>> solution;
};

/**
* @brief Result of get_result(): LP vs MIP is taken from the server response.
*/
template <typename i_t, typename f_t>
struct remote_result_t {
bool success = false;
std::string error_message;
bool is_mip = false;
std::unique_ptr<cpu_lp_solution_t<i_t, f_t>> lp_solution;
std::unique_ptr<cpu_mip_solution_t<i_t, f_t>> mip_solution;
};

/**
* @brief gRPC client for remote cuOpt solving
*
Expand Down Expand Up @@ -341,6 +353,16 @@ class grpc_client_t {
template <typename i_t, typename f_t>
remote_mip_result_t<i_t, f_t> get_mip_result(const std::string& job_id);

/**
* @brief Get result for a completed job; LP vs MIP comes from the server.
*
* Used by the Python async gRPC client today. Existing internal call sites
* still use get_lp_result / get_mip_result; they can migrate to get_result
* in a later change.
*/
template <typename i_t, typename f_t>
remote_result_t<i_t, f_t> get_result(const std::string& job_id);

/**
* @brief Cancel a running job
* @param job_id The job ID to cancel
Expand Down
212 changes: 212 additions & 0 deletions cpp/tests/linear_programming/grpc/grpc_client_test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -907,6 +907,218 @@ TEST_F(GrpcClientTest, ChunkedDownload_StartFails)
EXPECT_TRUE(lp_result.error_message.find("StartChunkedDownload") != std::string::npos);
}

// =============================================================================
// get_result (unified LP/MIP) Tests (Mock)
// =============================================================================

TEST_F(GrpcClientTest, GetResultUnified_UnaryLP)
{
EXPECT_CALL(*mock_stub_, CheckStatus(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::StatusRequest&,
cuopt::remote::StatusResponse* resp) {
resp->set_job_status(cuopt::remote::COMPLETED);
resp->set_result_size_bytes(64);
resp->set_max_message_bytes(256 * 1024 * 1024);
return grpc::Status::OK;
});

EXPECT_CALL(*mock_stub_, GetResult(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::GetResultRequest& req,
cuopt::remote::ResultResponse* resp) {
EXPECT_EQ(req.job_id(), "unified-lp-unary");
cuopt::remote::LPSolution solution;
solution.add_primal_solution(1.5);
solution.add_primal_solution(2.5);
solution.set_primal_objective(-464.753);
solution.set_lp_termination_status(cuopt::remote::PDLP_OPTIMAL);
resp->mutable_lp_solution()->CopyFrom(solution);
resp->set_status(cuopt::remote::SUCCESS);
return grpc::Status::OK;
});

auto result = client_->get_result<int32_t, double>("unified-lp-unary");

EXPECT_TRUE(result.success) << result.error_message;
EXPECT_FALSE(result.is_mip);
ASSERT_NE(result.lp_solution, nullptr);
EXPECT_EQ(result.mip_solution, nullptr);
EXPECT_NEAR(result.lp_solution->get_objective_value(), -464.753, 0.01);
}
Comment on lines +941 to +948

@coderabbitai coderabbitai Bot Aug 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Add a test for a unary response that carries no solution.

The new tests cover only success paths. get_result has a reachable error path: a unary ResultResponse with neither lp_solution nor mip_solution set. That input falls through the is_mip check into the LP branch and returns "GetResult succeeded but no LP solution in response". No test pins this behavior. Without it, a later refactor can return success == true with both solution pointers null, and grpc_python_client_t::result in cpp/src/grpc/client/cython_grpc_client.cpp then dereferences a null lp_solution.

Add one test that returns an empty ResultResponse and asserts success == false plus both pointers null.

💚 Proposed test 
TEST_F(GrpcClientTest, GetResultUnified_UnaryMissingSolution)
{
  EXPECT_CALL(*mock_stub_, CheckStatus(_, _, _))
    .WillOnce([](grpc::ClientContext*,
                 const cuopt::remote::StatusRequest&,
                 cuopt::remote::StatusResponse* resp) {
      resp->set_job_status(cuopt::remote::COMPLETED);
      resp->set_result_size_bytes(64);
      resp->set_max_message_bytes(256 * 1024 * 1024);
      return grpc::Status::OK;
    });

  EXPECT_CALL(*mock_stub_, GetResult(_, _, _))
    .WillOnce([](grpc::ClientContext*,
                 const cuopt::remote::GetResultRequest&,
                 cuopt::remote::ResultResponse* resp) {
      resp->set_status(cuopt::remote::SUCCESS);
      return grpc::Status::OK;
    });

  auto result = client_->get_result<int32_t, double>("unified-missing");

  EXPECT_FALSE(result.success);
  EXPECT_EQ(result.lp_solution, nullptr);
  EXPECT_EQ(result.mip_solution, nullptr);
  EXPECT_FALSE(result.error_message.empty());
}

As per path instructions: "Edge cases: empty, infeasible, unbounded, degenerate, singleton problems" and "When a bug fix lands, a regression test should cover the specific case".

🤖 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/grpc/grpc_client_test.cpp` around lines 941 -
948, Add a regression test near the existing unified unary get_result tests,
named GetResultUnified_UnaryMissingSolution, that mocks a completed CheckStatus
response and a successful GetResult response with neither solution field
populated. Call client_->get_result for the missing-solution job and assert
success is false, both lp_solution and mip_solution are null, and error_message
is non-empty.

Source: Path instructions


TEST_F(GrpcClientTest, GetResultUnified_UnaryMIP)
{
EXPECT_CALL(*mock_stub_, CheckStatus(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::StatusRequest&,
cuopt::remote::StatusResponse* resp) {
resp->set_job_status(cuopt::remote::COMPLETED);
resp->set_result_size_bytes(64);
resp->set_max_message_bytes(256 * 1024 * 1024);
return grpc::Status::OK;
});

EXPECT_CALL(*mock_stub_, GetResult(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::GetResultRequest& req,
cuopt::remote::ResultResponse* resp) {
EXPECT_EQ(req.job_id(), "unified-mip-unary");
cuopt::remote::MIPSolution solution;
solution.add_mip_solution(1.0);
solution.add_mip_solution(0.0);
solution.set_mip_objective(42.0);
solution.set_mip_termination_status(cuopt::remote::MIP_OPTIMAL);
resp->mutable_mip_solution()->CopyFrom(solution);
resp->set_status(cuopt::remote::SUCCESS);
return grpc::Status::OK;
});

auto result = client_->get_result<int32_t, double>("unified-mip-unary");

EXPECT_TRUE(result.success) << result.error_message;
EXPECT_TRUE(result.is_mip);
ASSERT_NE(result.mip_solution, nullptr);
EXPECT_EQ(result.lp_solution, nullptr);
EXPECT_DOUBLE_EQ(result.mip_solution->get_objective_value(), 42.0);
}

TEST_F(GrpcClientTest, GetResultUnified_ChunkedLP_FallbackOnResourceExhausted)
{
EXPECT_CALL(*mock_stub_, CheckStatus(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::StatusRequest&,
cuopt::remote::StatusResponse* resp) {
resp->set_job_status(cuopt::remote::COMPLETED);
resp->set_result_size_bytes(500);
resp->set_max_message_bytes(256 * 1024 * 1024);
return grpc::Status::OK;
});

EXPECT_CALL(*mock_stub_, GetResult(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::GetResultRequest&,
cuopt::remote::ResultResponse*) {
return grpc::Status(grpc::StatusCode::RESOURCE_EXHAUSTED, "Too large");
});

EXPECT_CALL(*mock_stub_, StartChunkedDownload(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::StartChunkedDownloadRequest&,
cuopt::remote::StartChunkedDownloadResponse* resp) {
resp->set_download_id("dl-unified-lp");
auto* h = resp->mutable_header();
h->set_problem_category(cuopt::remote::LP);
h->set_lp_termination_status(cuopt::remote::PDLP_OPTIMAL);
h->set_primal_objective(-464.753);
auto* arr = h->add_arrays();
arr->set_field_id(cuopt::remote::RESULT_PRIMAL_SOLUTION);
arr->set_total_elements(2);
arr->set_element_size_bytes(8);
resp->set_max_message_bytes(4 * 1024 * 1024);
return grpc::Status::OK;
});

EXPECT_CALL(*mock_stub_, GetResultChunk(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::GetResultChunkRequest& req,
cuopt::remote::GetResultChunkResponse* resp) {
EXPECT_EQ(req.download_id(), "dl-unified-lp");
EXPECT_EQ(req.field_id(), cuopt::remote::RESULT_PRIMAL_SOLUTION);
resp->set_download_id("dl-unified-lp");
resp->set_field_id(req.field_id());
resp->set_element_offset(0);
resp->set_elements_in_chunk(2);
double vals[2] = {1.5, 2.5};
resp->set_data(reinterpret_cast<const char*>(vals), sizeof(vals));
return grpc::Status::OK;
});

EXPECT_CALL(*mock_stub_, FinishChunkedDownload(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::FinishChunkedDownloadRequest& req,
cuopt::remote::FinishChunkedDownloadResponse* resp) {
resp->set_download_id(req.download_id());
return grpc::Status::OK;
});

auto result = client_->get_result<int32_t, double>("unified-lp-chunked");

EXPECT_TRUE(result.success) << result.error_message;
EXPECT_FALSE(result.is_mip);
ASSERT_NE(result.lp_solution, nullptr);
EXPECT_EQ(result.mip_solution, nullptr);
EXPECT_NEAR(result.lp_solution->get_objective_value(), -464.753, 0.01);
}

TEST_F(GrpcClientTest, GetResultUnified_ChunkedMIP_FallbackOnResourceExhausted)
{
EXPECT_CALL(*mock_stub_, CheckStatus(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::StatusRequest&,
cuopt::remote::StatusResponse* resp) {
resp->set_job_status(cuopt::remote::COMPLETED);
resp->set_result_size_bytes(500);
resp->set_max_message_bytes(256 * 1024 * 1024);
return grpc::Status::OK;
});

EXPECT_CALL(*mock_stub_, GetResult(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::GetResultRequest&,
cuopt::remote::ResultResponse*) {
return grpc::Status(grpc::StatusCode::RESOURCE_EXHAUSTED, "Too large");
});

EXPECT_CALL(*mock_stub_, StartChunkedDownload(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::StartChunkedDownloadRequest&,
cuopt::remote::StartChunkedDownloadResponse* resp) {
resp->set_download_id("dl-unified-mip");
auto* h = resp->mutable_header();
h->set_problem_category(cuopt::remote::MIP);
h->set_mip_termination_status(cuopt::remote::MIP_OPTIMAL);
h->set_mip_objective(42.0);
auto* arr = h->add_arrays();
arr->set_field_id(cuopt::remote::RESULT_MIP_SOLUTION);
arr->set_total_elements(2);
arr->set_element_size_bytes(8);
resp->set_max_message_bytes(4 * 1024 * 1024);
return grpc::Status::OK;
});

EXPECT_CALL(*mock_stub_, GetResultChunk(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::GetResultChunkRequest& req,
cuopt::remote::GetResultChunkResponse* resp) {
EXPECT_EQ(req.download_id(), "dl-unified-mip");
EXPECT_EQ(req.field_id(), cuopt::remote::RESULT_MIP_SOLUTION);
resp->set_download_id("dl-unified-mip");
resp->set_field_id(req.field_id());
resp->set_element_offset(0);
resp->set_elements_in_chunk(2);
double vals[2] = {1.0, 0.0};
resp->set_data(reinterpret_cast<const char*>(vals), sizeof(vals));
return grpc::Status::OK;
});

EXPECT_CALL(*mock_stub_, FinishChunkedDownload(_, _, _))
.WillOnce([](grpc::ClientContext*,
const cuopt::remote::FinishChunkedDownloadRequest& req,
cuopt::remote::FinishChunkedDownloadResponse* resp) {
resp->set_download_id(req.download_id());
return grpc::Status::OK;
});

auto result = client_->get_result<int32_t, double>("unified-mip-chunked");

EXPECT_TRUE(result.success) << result.error_message;
EXPECT_TRUE(result.is_mip);
ASSERT_NE(result.mip_solution, nullptr);
EXPECT_EQ(result.lp_solution, nullptr);
EXPECT_DOUBLE_EQ(result.mip_solution->get_objective_value(), 42.0);
}

// =============================================================================
// Helper: Build minimal test problems
// =============================================================================
Expand Down
2 changes: 1 addition & 1 deletion python/cuopt/cuopt/grpc/linear_programming/grpc_client.pxd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ cdef extern from "cuopt/grpc/cython_grpc_client.hpp" namespace "cuopt::cython":
grpc_status_result_t wait(const string& job_id, int timeout_seconds) except +
bint cancel(const string& job_id, string& error_out) except +
bint delete_job(const string& job_id, string& error_out) except +
grpc_result_outcome_t result(const string& job_id, bint is_mip) except +
grpc_result_outcome_t result(const string& job_id) except +
grpc_logs_result_t fetch_logs(const string& job_id, long long from_byte) except +
bint stream_logs(
const string& job_id,
Expand Down
Loading
Loading