diff --git a/cpp/src/grpc/codegen/FIELD_REGISTRY_REFERENCE.md b/cpp/src/grpc/codegen/FIELD_REGISTRY_REFERENCE.md index c1c36180d6..bff662a6fb 100644 --- a/cpp/src/grpc/codegen/FIELD_REGISTRY_REFERENCE.md +++ b/cpp/src/grpc/codegen/FIELD_REGISTRY_REFERENCE.md @@ -242,9 +242,10 @@ section: (the setter / assignment is skipped). This is the case the flag is designed for: without `optional:`, an external client that omits the field silently overwrites the C++ default with the proto3 zero. -- **Problem-chunked**: currently *not honored* — the chunked path reads from - the hand-written `ChunkedProblemHeader` message which does not declare - `optional` on its fields. Tracked for unification (see §6). +- **Problem-chunked**: the C++ field's in-class default is preserved, provided + the matching field in the hand-written `ChunkedProblemHeader` message is + also declared `optional`. The generator emits a `has_X()` guard for registry + fields marked `optional`. - **Solution**: cosmetic. Solutions are constructor-built each call; there's no pre-existing default to preserve. The `optional` keyword adds `has_X()` to the proto for client-side presence detection, but the from-proto path still diff --git a/cpp/src/grpc/codegen/field_registry.yaml b/cpp/src/grpc/codegen/field_registry.yaml index 1981af5091..8ae0ff26b8 100644 --- a/cpp/src/grpc/codegen/field_registry.yaml +++ b/cpp/src/grpc/codegen/field_registry.yaml @@ -819,6 +819,10 @@ optimization_problem: getter: get_sense() - objective_scaling_factor: field_num: 4 + # C++ defaults to 1 while an absent proto3 scalar reads as 0. Track + # presence so external clients may omit the field without changing the + # objective to an unsupported zero scaling factor. + optional: true - objective_offset: field_num: 5 diff --git a/cpp/src/grpc/codegen/generate_conversions.py b/cpp/src/grpc/codegen/generate_conversions.py index 1cd4458230..b7088a677e 100644 --- a/cpp/src/grpc/codegen/generate_conversions.py +++ b/cpp/src/grpc/codegen/generate_conversions.py @@ -2420,14 +2420,14 @@ def _gen_chunked_header_to_problem(registry, indent=" "): ind = indent lines = [] - # `ChunkedProblemHeader` is hand-written (see cuopt_remote_service.proto); - # it does not declare `optional` for any field, so we pass has_check=None - # to suppress the has_X() guard. `sentinel:` still applies — the wire - # mapping must stay consistent with the unary path. + # `ChunkedProblemHeader` is hand-written (see cuopt_remote_service.proto). + # Fields marked optional in the registry must also be declared optional in + # that message so the chunked path can preserve non-zero C++ defaults. for entry in obj.get("scalars", []): f = parse_field(entry) pname = _proto_cpp_name(f["name"]) setter = _default_problem_setter(f) + has_check = f"header.has_{pname}()" if f.get("optional") else None lines.extend( emit_scalar_from_proto_assign( lambda v, s=setter: f"cpu_problem.{s}({v});", @@ -2435,7 +2435,7 @@ def _gen_chunked_header_to_problem(registry, indent=" "): f, registry, ind, - has_check=None, + has_check=has_check, ) ) diff --git a/cpp/src/grpc/codegen/generated/cuopt_remote_data.proto b/cpp/src/grpc/codegen/generated/cuopt_remote_data.proto index 6c3931664e..a79a29909e 100644 --- a/cpp/src/grpc/codegen/generated/cuopt_remote_data.proto +++ b/cpp/src/grpc/codegen/generated/cuopt_remote_data.proto @@ -137,7 +137,7 @@ message OptimizationProblem { string problem_name = 1; string objective_name = 2; bool maximize = 3; - double objective_scaling_factor = 4; + optional double objective_scaling_factor = 4; double objective_offset = 5; repeated string variable_names = 7; repeated string row_names = 8; diff --git a/cpp/src/grpc/codegen/generated/generated_chunked_header_to_problem.inc b/cpp/src/grpc/codegen/generated/generated_chunked_header_to_problem.inc index e5ef0a3365..6558b31d0f 100644 --- a/cpp/src/grpc/codegen/generated/generated_chunked_header_to_problem.inc +++ b/cpp/src/grpc/codegen/generated/generated_chunked_header_to_problem.inc @@ -5,7 +5,9 @@ cpu_problem.set_problem_name(header.problem_name()); cpu_problem.set_objective_name(header.objective_name()); cpu_problem.set_maximize(header.maximize()); - cpu_problem.set_objective_scaling_factor(header.objective_scaling_factor()); + if (header.has_objective_scaling_factor()) { + cpu_problem.set_objective_scaling_factor(header.objective_scaling_factor()); + } cpu_problem.set_objective_offset(header.objective_offset()); if (header.variable_names_size() > 0) { diff --git a/cpp/src/grpc/codegen/generated/generated_proto_to_problem.inc b/cpp/src/grpc/codegen/generated/generated_proto_to_problem.inc index 7f99650caf..58271c0577 100644 --- a/cpp/src/grpc/codegen/generated/generated_proto_to_problem.inc +++ b/cpp/src/grpc/codegen/generated/generated_proto_to_problem.inc @@ -5,7 +5,9 @@ cpu_problem.set_problem_name(pb_problem.problem_name()); cpu_problem.set_objective_name(pb_problem.objective_name()); cpu_problem.set_maximize(pb_problem.maximize()); - cpu_problem.set_objective_scaling_factor(pb_problem.objective_scaling_factor()); + if (pb_problem.has_objective_scaling_factor()) { + cpu_problem.set_objective_scaling_factor(pb_problem.objective_scaling_factor()); + } cpu_problem.set_objective_offset(pb_problem.objective_offset()); if (pb_problem.a_offsets_size() > 0) { diff --git a/cpp/src/grpc/cuopt_remote_service.proto b/cpp/src/grpc/cuopt_remote_service.proto index d8d617af2f..a9795097a4 100644 --- a/cpp/src/grpc/cuopt_remote_service.proto +++ b/cpp/src/grpc/cuopt_remote_service.proto @@ -118,7 +118,7 @@ message ChunkedProblemHeader { // Problem scalars bool maximize = 2; - double objective_scaling_factor = 3; + optional double objective_scaling_factor = 3; double objective_offset = 4; string problem_name = 5; string objective_name = 6; diff --git a/cpp/tests/linear_programming/grpc/grpc_client_test.cpp b/cpp/tests/linear_programming/grpc/grpc_client_test.cpp index e468cef694..0d3b7301cc 100644 --- a/cpp/tests/linear_programming/grpc/grpc_client_test.cpp +++ b/cpp/tests/linear_programming/grpc/grpc_client_test.cpp @@ -1857,6 +1857,53 @@ TEST(MapperRoundtrip, ProblemWithVariableTypes) EXPECT_DOUBLE_EQ(restored_obj[2], 3.0); } +TEST(MapperRoundtrip, UnaryProblemObjectiveScalingPresence) +{ + cuopt::remote::OptimizationProblem omitted; + omitted.add_c(0.0); + omitted.add_variable_lower_bounds(0.0); + omitted.add_variable_upper_bounds(1.0); + ASSERT_FALSE(omitted.has_objective_scaling_factor()); + + cpu_optimization_problem_t restored_default; + map_proto_to_problem(omitted, restored_default); + EXPECT_DOUBLE_EQ(restored_default.get_objective_scaling_factor(), 1.0); + + cpu_optimization_problem_t orig; + const std::vector objective{0.0}; + const std::vector lower_bound{0.0}; + const std::vector upper_bound{1.0}; + orig.set_objective_coefficients(objective.data(), objective.size()); + orig.set_variable_lower_bounds(lower_bound.data(), lower_bound.size()); + orig.set_variable_upper_bounds(upper_bound.data(), upper_bound.size()); + orig.set_objective_scaling_factor(2.5); + cuopt::remote::OptimizationProblem present; + map_problem_to_proto(orig, &present); + ASSERT_TRUE(present.has_objective_scaling_factor()); + EXPECT_DOUBLE_EQ(present.objective_scaling_factor(), 2.5); + + cpu_optimization_problem_t restored_present; + map_proto_to_problem(present, restored_present); + EXPECT_DOUBLE_EQ(restored_present.get_objective_scaling_factor(), 2.5); +} + +TEST(MapperRoundtrip, ChunkedProblemObjectiveScalingPresence) +{ + cuopt::remote::ChunkedProblemHeader omitted; + ASSERT_FALSE(omitted.has_objective_scaling_factor()); + + cpu_optimization_problem_t restored_default; + map_chunked_header_to_problem(omitted, restored_default); + EXPECT_DOUBLE_EQ(restored_default.get_objective_scaling_factor(), 1.0); + + omitted.set_objective_scaling_factor(2.5); + ASSERT_TRUE(omitted.has_objective_scaling_factor()); + + cpu_optimization_problem_t restored_present; + map_chunked_header_to_problem(omitted, restored_present); + EXPECT_DOUBLE_EQ(restored_present.get_objective_scaling_factor(), 2.5); +} + TEST(MapperRoundtrip, MIPSolutionAllFields) { std::vector sol_vec = {1.0, 0.0, 1.0, 0.0, 1.0};