From 3756aaf7bcf53d959c70b371bb331b4c571f5b45 Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Tue, 28 Jul 2026 16:55:53 -0500 Subject: [PATCH 1/3] feat(c-api): add cuOptSetLogCallback and cuOptSetLogLevel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds two new C API functions to address #1536 and #184: - cuOptSetLogCallback(settings, callback, user_data): registers a per-line log callback. Invoked once per log line in addition to any console/file sink already enabled. Pass NULL to clear. - cuOptSetLogLevel(settings, level): overrides log verbosity using the new CUOPT_LOG_LEVEL_* constants (TRACE…OFF) in constants.h. Internally, the callback is stored in solver_settings_handle_t and installed as a rapids_logger::callback_sink_mt via a RAII scope guard in cuOptSolve, so init_logger_t picks it up without any changes to the C++ internal solver-settings structs. The pending globals are protected by the existing g_guard_mutex that already serialises init_logger_t construction. Co-Authored-By: Claude Sonnet 4.6 (1M context) Signed-off-by: Ramakrishna Prabhu --- .../mathematical_optimization/constants.h | 9 ++++ .../cuopt/mathematical_optimization/cuopt_c.h | 39 +++++++++++++++ cpp/src/pdlp/cuopt_c.cpp | 49 +++++++++++++++++++ cpp/src/utilities/logger.cpp | 45 +++++++++++++++++ cpp/src/utilities/logger.hpp | 20 ++++++++ 5 files changed, 162 insertions(+) diff --git a/cpp/include/cuopt/mathematical_optimization/constants.h b/cpp/include/cuopt/mathematical_optimization/constants.h index 9592389dea..d2ba9fa38c 100644 --- a/cpp/include/cuopt/mathematical_optimization/constants.h +++ b/cpp/include/cuopt/mathematical_optimization/constants.h @@ -146,6 +146,15 @@ /* @brief QCQP (barrier) scaling hyper-parameters */ #define CUOPT_QCQP_HYPER_RUIZ_EQUILIBRATION "qcqp_hyper_ruiz_equilibration" +/* @brief Log level constants for cuOptSetLogLevel */ +#define CUOPT_LOG_LEVEL_TRACE 0 +#define CUOPT_LOG_LEVEL_DEBUG 1 +#define CUOPT_LOG_LEVEL_INFO 2 +#define CUOPT_LOG_LEVEL_WARN 3 +#define CUOPT_LOG_LEVEL_ERROR 4 +#define CUOPT_LOG_LEVEL_CRITICAL 5 +#define CUOPT_LOG_LEVEL_OFF 6 + /* @brief MIP determinism mode constants */ #define CUOPT_MODE_OPPORTUNISTIC 0 #define CUOPT_MODE_DETERMINISTIC 1 diff --git a/cpp/include/cuopt/mathematical_optimization/cuopt_c.h b/cpp/include/cuopt/mathematical_optimization/cuopt_c.h index b135fed72a..d6382e03bc 100644 --- a/cpp/include/cuopt/mathematical_optimization/cuopt_c.h +++ b/cpp/include/cuopt/mathematical_optimization/cuopt_c.h @@ -823,6 +823,45 @@ cuopt_int_t cuOptGetFloatParameter(cuOptSolverSettings settings, const char* parameter_name, cuopt_float_t* parameter_value); +/** + * @brief Type of callback invoked once per log line emitted by the solver. + * + * @param level Log level (one of CUOPT_LOG_LEVEL_*). + * @param message Null-terminated log line without trailing newline. + * @param user_data Opaque pointer passed to cuOptSetLogCallback. + * + * @note The callback is invoked from the solver thread. Do not call back into + * cuOpt from inside the callback. + */ +typedef void (*cuOptLogCallback)(int level, const char* message, void* user_data); + +/** + * @brief Register a callback to receive solver log messages. + * + * The callback is invoked once per log line. It is called in addition to any + * file or console sink already enabled via ``log_to_console`` / ``log_file`` + * parameters. Pass NULL to remove a previously registered callback. + * + * @param[in] settings The solver settings object. + * @param[in] callback Callback function, or NULL to clear. + * @param[in] user_data Opaque pointer forwarded to the callback unchanged. + * + * @return A status code indicating success or failure. + */ +cuopt_int_t cuOptSetLogCallback(cuOptSolverSettings settings, + cuOptLogCallback callback, + void* user_data); + +/** + * @brief Set the solver log verbosity level. + * + * @param[in] settings The solver settings object. + * @param[in] level One of CUOPT_LOG_LEVEL_TRACE … CUOPT_LOG_LEVEL_OFF. + * + * @return A status code indicating success or failure. + */ +cuopt_int_t cuOptSetLogLevel(cuOptSolverSettings settings, int level); + /** * @brief Type of callback for receiving incumbent MIP solutions with user context. * diff --git a/cpp/src/pdlp/cuopt_c.cpp b/cpp/src/pdlp/cuopt_c.cpp index a813abf71f..c7556d2085 100644 --- a/cpp/src/pdlp/cuopt_c.cpp +++ b/cpp/src/pdlp/cuopt_c.cpp @@ -89,6 +89,11 @@ struct solver_settings_handle_t { ~solver_settings_handle_t() { delete settings; } solver_settings_t* settings; std::vector> callbacks; + // Log callback registered via cuOptSetLogCallback + cuOptLogCallback log_callback{nullptr}; + void* log_callback_user_data{nullptr}; + // Log level override registered via cuOptSetLogLevel (-1 = use default) + int log_level{-1}; }; solver_settings_handle_t* get_settings_handle(cuOptSolverSettings settings) @@ -1069,6 +1074,27 @@ cuopt_int_t cuOptSetMIPSetSolutionCallback(cuOptSolverSettings settings, return CUOPT_SUCCESS; } +cuopt_int_t cuOptSetLogCallback(cuOptSolverSettings settings, + cuOptLogCallback callback, + void* user_data) +{ + if (settings == nullptr) { return CUOPT_INVALID_ARGUMENT; } + solver_settings_handle_t* handle = get_settings_handle(settings); + handle->log_callback = callback; + handle->log_callback_user_data = user_data; + return CUOPT_SUCCESS; +} + +cuopt_int_t cuOptSetLogLevel(cuOptSolverSettings settings, int level) +{ + if (settings == nullptr) { return CUOPT_INVALID_ARGUMENT; } + if (level < CUOPT_LOG_LEVEL_TRACE || level > CUOPT_LOG_LEVEL_OFF) { + return CUOPT_INVALID_ARGUMENT; + } + get_settings_handle(settings)->log_level = level; + return CUOPT_SUCCESS; +} + cuopt_int_t cuOptSetInitialPrimalSolution(cuOptSolverSettings settings, const cuopt_float_t* primal_solution, cuopt_int_t num_variables) @@ -1145,6 +1171,29 @@ cuopt_int_t cuOptSolve(cuOptOptimizationProblem problem, if (settings == nullptr) { return CUOPT_INVALID_ARGUMENT; } if (solution_ptr == nullptr) { return CUOPT_INVALID_ARGUMENT; } + // Install user log callback / level so init_logger_t inside the solver picks them up. + // The RAII guard clears them on scope exit (whether by return or exception). + solver_settings_handle_t* handle = get_settings_handle(settings); + struct log_scope_guard_t { + bool has_callback; + bool has_level; + ~log_scope_guard_t() + { + if (has_callback) { cuopt::clear_pending_log_callback(); } + if (has_level) { cuopt::clear_pending_log_level(); } + } + } log_scope{false, false}; + + if (handle->log_callback) { + // cuOptLogCallback and log_callback_with_data_t share the same signature. + cuopt::set_pending_log_callback(handle->log_callback, handle->log_callback_user_data); + log_scope.has_callback = true; + } + if (handle->log_level >= 0) { + cuopt::set_pending_log_level(handle->log_level); + log_scope.has_level = true; + } + problem_and_stream_view_t* problem_and_stream_view = static_cast(problem); diff --git a/cpp/src/utilities/logger.cpp b/cpp/src/utilities/logger.cpp index 217f9c64cb..f0e04d8895 100644 --- a/cpp/src/utilities/logger.cpp +++ b/cpp/src/utilities/logger.cpp @@ -146,6 +146,43 @@ struct logger_config_guard { static std::weak_ptr g_active_guard; static std::mutex g_guard_mutex; +// Pending user log callback set by the C API before a solve. +// Accessed under g_guard_mutex. +static log_callback_with_data_t g_pending_callback = nullptr; +static void* g_pending_callback_data = nullptr; +static int g_pending_log_level = -1; // -1 = use compiled default + +static void user_log_bridge(int lvl, const char* msg) +{ + if (g_pending_callback) { g_pending_callback(lvl, msg, g_pending_callback_data); } +} + +void set_pending_log_callback(log_callback_with_data_t cb, void* user_data) +{ + std::lock_guard lock(g_guard_mutex); + g_pending_callback = cb; + g_pending_callback_data = user_data; +} + +void clear_pending_log_callback() +{ + std::lock_guard lock(g_guard_mutex); + g_pending_callback = nullptr; + g_pending_callback_data = nullptr; +} + +void set_pending_log_level(int level) +{ + std::lock_guard lock(g_guard_mutex); + g_pending_log_level = level; +} + +void clear_pending_log_level() +{ + std::lock_guard lock(g_guard_mutex); + g_pending_log_level = -1; +} + init_logger_t::init_logger_t(std::string log_file, bool log_to_console) { std::lock_guard lock(g_guard_mutex); @@ -169,6 +206,10 @@ init_logger_t::init_logger_t(std::string log_file, bool log_to_console) std::make_shared(log_file, true)); cuopt::default_logger().flush_on(rapids_logger::level_enum::debug); } + if (g_pending_callback) { + cuopt::default_logger().sinks().push_back( + std::make_shared(user_log_bridge)); + } #if CUOPT_LOG_ACTIVE_LEVEL >= RAPIDS_LOGGER_LOG_LEVEL_INFO cuopt::default_logger().set_pattern("%v"); @@ -176,6 +217,10 @@ init_logger_t::init_logger_t(std::string log_file, bool log_to_console) cuopt::default_logger().set_pattern(cuopt::default_pattern()); #endif + if (g_pending_log_level >= 0) { + cuopt::default_logger().set_level(static_cast(g_pending_log_level)); + } + // Extract messages from the global buffer and log to the default logger auto buffered_messages = global_log_buffer().drain_all(); for (const auto& entry : buffered_messages) { diff --git a/cpp/src/utilities/logger.hpp b/cpp/src/utilities/logger.hpp index f3a5bf6f2f..e7ece939b9 100644 --- a/cpp/src/utilities/logger.hpp +++ b/cpp/src/utilities/logger.hpp @@ -36,6 +36,26 @@ rapids_logger::logger& default_logger(); */ void reset_default_logger(); +// C-compatible log callback type: void callback(int level, const char* msg, void* user_data) +// Matches cuOptLogCallback in cuopt_c.h — layout-compatible, no dependency on that header. +using log_callback_with_data_t = void (*)(int level, const char* message, void* user_data); + +/** + * @brief Install a user log callback to be picked up by the next init_logger_t. + * + * Must be called before the init_logger_t that starts the targeted solve. + * Protected by the same mutex as init_logger_t so it is safe to call from + * any thread, but do not call from inside the callback itself. + */ +void set_pending_log_callback(log_callback_with_data_t cb, void* user_data); +void clear_pending_log_callback(); + +/** + * @brief Override the log level for the next init_logger_t. Pass -1 to restore the default. + */ +void set_pending_log_level(int level); +void clear_pending_log_level(); + // Ref-counted logger initializer class init_logger_t { // Using shared_ptr for ref-counting From a1ce1149c7d489c048855138261b5e0c2526fe5f Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Wed, 29 Jul 2026 13:44:57 -0500 Subject: [PATCH 2/3] fix/test(c-api): fix log callback race condition and add tests Fix a data race flagged in PR review: user_log_bridge was reading g_pending_callback (a mutable global) without any lock, creating a potential race against clear_pending_log_callback() called from cuOptSolve's RAII scope guard. Fix: capture the callback + user_data immutably into the logger_config_guard at init_logger_t construction time. The bridge now reads g_active_log_callback, a stable pointer into the guard's owned state. The pointer is only nulled after reset_default_logger() removes the sink (which blocks until all in-flight bridge calls complete), so there is no race window. Also adds a @warning to cuOptLogCallback documenting that log message formatting is not a stable API and the callback is intended for display purposes, not for parsing solver events programmatically. Tests added (c_api_test.c): - log_callback: verifies callback is invoked and user_data is forwarded - log_callback_cleared: verifies NULL callback produces no calls - log_level_off: verifies CUOPT_LOG_LEVEL_OFF silences all messages - log_level_invalid: verifies out-of-range levels are rejected Co-Authored-By: Claude Sonnet 4.6 (1M context) Signed-off-by: Ramakrishna Prabhu --- .../cuopt/mathematical_optimization/cuopt_c.h | 4 + cpp/src/utilities/logger.cpp | 49 ++++-- .../c_api_tests/c_api_test.c | 155 ++++++++++++++++++ .../c_api_tests/c_api_tests.cpp | 8 + .../c_api_tests/c_api_tests.h | 4 + 5 files changed, 209 insertions(+), 11 deletions(-) diff --git a/cpp/include/cuopt/mathematical_optimization/cuopt_c.h b/cpp/include/cuopt/mathematical_optimization/cuopt_c.h index d6382e03bc..32628d63c9 100644 --- a/cpp/include/cuopt/mathematical_optimization/cuopt_c.h +++ b/cpp/include/cuopt/mathematical_optimization/cuopt_c.h @@ -832,6 +832,10 @@ cuopt_int_t cuOptGetFloatParameter(cuOptSolverSettings settings, * * @note The callback is invoked from the solver thread. Do not call back into * cuOpt from inside the callback. + * @warning Log message formatting is not part of the stable API and may change + * between releases. The callback is intended for display purposes (GUI integration, + * log forwarding, stdout capture) — do not parse message content for programmatic + * control flow. */ typedef void (*cuOptLogCallback)(int level, const char* message, void* user_data); diff --git a/cpp/src/utilities/logger.cpp b/cpp/src/utilities/logger.cpp index f0e04d8895..81e7275c0e 100644 --- a/cpp/src/utilities/logger.cpp +++ b/cpp/src/utilities/logger.cpp @@ -137,24 +137,48 @@ void reset_default_logger() default_logger().flush_on(rapids_logger::level_enum::debug); } -// Guard object whose destructor resets the logger +// Forward declarations needed by logger_config_guard destructor. +static std::mutex g_guard_mutex; +static const struct captured_log_callback_t* g_active_log_callback; + +// Captured (immutable) callback state owned by the active logger guard. +struct captured_log_callback_t { + log_callback_with_data_t callback; + void* user_data; +}; + +// Guard object whose destructor resets the logger. +// Owns the captured callback state to guarantee its lifetime. struct logger_config_guard { - ~logger_config_guard() { cuopt::reset_default_logger(); } + std::unique_ptr callback_state; + ~logger_config_guard() + { + cuopt::reset_default_logger(); // removes the sink; blocks until in-flight log calls finish + std::lock_guard lock(g_guard_mutex); + g_active_log_callback = nullptr; // safe: the sink (and the bridge) are already gone + } }; // Weak reference to detect if any init_logger_t instance is still alive static std::weak_ptr g_active_guard; -static std::mutex g_guard_mutex; -// Pending user log callback set by the C API before a solve. -// Accessed under g_guard_mutex. -static log_callback_with_data_t g_pending_callback = nullptr; -static void* g_pending_callback_data = nullptr; -static int g_pending_log_level = -1; // -1 = use compiled default +// g_active_log_callback: written only under g_guard_mutex (at guard create/destroy time). +// Read lock-free by user_log_bridge — safe because the bridge is only reachable +// while the sink is alive, and the sink is removed (in reset_default_logger) before +// this pointer is cleared. + +// Pending user log callback/level set by the C API before cuOptSolve. +// Consumed once (under g_guard_mutex) by init_logger_t to build the guard state. +static log_callback_with_data_t g_pending_callback = nullptr; +static void* g_pending_callback_data = nullptr; +static int g_pending_log_level = -1; // -1 = use compiled default static void user_log_bridge(int lvl, const char* msg) { - if (g_pending_callback) { g_pending_callback(lvl, msg, g_pending_callback_data); } + // g_active_log_callback is stable for the duration of any bridge call: + // it points into the guard's callback_state, which outlives the sink. + const captured_log_callback_t* state = g_active_log_callback; + if (state) { state->callback(lvl, msg, state->user_data); } } void set_pending_log_callback(log_callback_with_data_t cb, void* user_data) @@ -206,7 +230,12 @@ init_logger_t::init_logger_t(std::string log_file, bool log_to_console) std::make_shared(log_file, true)); cuopt::default_logger().flush_on(rapids_logger::level_enum::debug); } + // Capture pending callback into the guard so the bridge reads stable (immutable) state. + auto guard = std::make_shared(); if (g_pending_callback) { + guard->callback_state = + std::make_unique(captured_log_callback_t{g_pending_callback, g_pending_callback_data}); + g_active_log_callback = guard->callback_state.get(); cuopt::default_logger().sinks().push_back( std::make_shared(user_log_bridge)); } @@ -227,8 +256,6 @@ init_logger_t::init_logger_t(std::string log_file, bool log_to_console) cuopt::default_logger().log(entry.level, entry.msg.c_str()); } - // Create guard and store weak reference for future instances to find - auto guard = std::make_shared(); g_active_guard = guard; guard_ = guard; } diff --git a/cpp/tests/linear_programming/c_api_tests/c_api_test.c b/cpp/tests/linear_programming/c_api_tests/c_api_test.c index 31bd18d0ef..8c77016835 100644 --- a/cpp/tests/linear_programming/c_api_tests/c_api_test.c +++ b/cpp/tests/linear_programming/c_api_tests/c_api_test.c @@ -292,6 +292,161 @@ cuopt_int_t test_mip_get_callbacks_only() { return test_mip_callbacks_internal(0 cuopt_int_t test_mip_get_set_callbacks() { return test_mip_callbacks_internal(1); } +/* ------------------------------------------------------------------------- + * Log callback tests + * Use a small LP (1 variable, 1 constraint) so no GPU / dataset is needed. + * ------------------------------------------------------------------------- */ + +/* Build a trivial 1-variable LP: min x s.t. x >= 1, 0 <= x <= inf */ +static cuopt_int_t make_trivial_lp(cuOptOptimizationProblem* problem_out, + cuOptSolverSettings* settings_out) +{ + cuopt_float_t obj[] = {1.0}; + cuopt_int_t row_off[] = {0, 1}; + cuopt_int_t col_idx[] = {0}; + cuopt_float_t coeff[] = {1.0}; + char sense[] = {CUOPT_GREATER_THAN}; + cuopt_float_t rhs[] = {1.0}; + cuopt_float_t lb[] = {0.0}; + cuopt_float_t ub[] = {1e30}; + char vtype[] = {CUOPT_CONTINUOUS}; + + cuopt_int_t status = + cuOptCreateProblem(1, 1, CUOPT_MINIMIZE, 0.0, obj, row_off, col_idx, coeff, + sense, rhs, lb, ub, vtype, problem_out); + if (status != CUOPT_SUCCESS) return status; + return cuOptCreateSolverSettings(settings_out); +} + +typedef struct { + int calls; + void* received_user_data; +} log_cb_context_t; + +static void counting_log_callback(int level, const char* message, void* user_data) +{ + (void)level; + (void)message; + log_cb_context_t* ctx = (log_cb_context_t*)user_data; + ctx->calls++; + ctx->received_user_data = user_data; +} + +cuopt_int_t test_log_callback(void) +{ + cuOptOptimizationProblem problem = NULL; + cuOptSolverSettings settings = NULL; + cuOptSolution solution = NULL; + log_cb_context_t ctx = {0, NULL}; + cuopt_int_t status = make_trivial_lp(&problem, &settings); + if (status != CUOPT_SUCCESS) goto DONE; + + status = cuOptSetLogCallback(settings, counting_log_callback, &ctx); + if (status != CUOPT_SUCCESS) goto DONE; + + status = cuOptSolve(problem, settings, &solution); + if (status != CUOPT_SUCCESS) goto DONE; + + if (ctx.calls < 1) { + printf("Expected log callback to be called at least once; got %d calls\n", ctx.calls); + status = CUOPT_INVALID_ARGUMENT; + goto DONE; + } + if (ctx.received_user_data != &ctx) { + printf("user_data pointer was not forwarded correctly\n"); + status = CUOPT_INVALID_ARGUMENT; + goto DONE; + } + +DONE: + cuOptDestroyProblem(&problem); + cuOptDestroySolverSettings(&settings); + cuOptDestroySolution(&solution); + return status; +} + +cuopt_int_t test_log_callback_cleared(void) +{ + cuOptOptimizationProblem problem = NULL; + cuOptSolverSettings settings = NULL; + cuOptSolution solution = NULL; + log_cb_context_t ctx = {0, NULL}; + cuopt_int_t status = make_trivial_lp(&problem, &settings); + if (status != CUOPT_SUCCESS) goto DONE; + + /* Register then immediately clear the callback */ + status = cuOptSetLogCallback(settings, counting_log_callback, &ctx); + if (status != CUOPT_SUCCESS) goto DONE; + status = cuOptSetLogCallback(settings, NULL, NULL); + if (status != CUOPT_SUCCESS) goto DONE; + + status = cuOptSolve(problem, settings, &solution); + if (status != CUOPT_SUCCESS) goto DONE; + + if (ctx.calls != 0) { + printf("Expected 0 callback calls after clearing; got %d\n", ctx.calls); + status = CUOPT_INVALID_ARGUMENT; + goto DONE; + } + +DONE: + cuOptDestroyProblem(&problem); + cuOptDestroySolverSettings(&settings); + cuOptDestroySolution(&solution); + return status; +} + +cuopt_int_t test_log_level_off(void) +{ + cuOptOptimizationProblem problem = NULL; + cuOptSolverSettings settings = NULL; + cuOptSolution solution = NULL; + log_cb_context_t ctx = {0, NULL}; + cuopt_int_t status = make_trivial_lp(&problem, &settings); + if (status != CUOPT_SUCCESS) goto DONE; + + status = cuOptSetLogCallback(settings, counting_log_callback, &ctx); + if (status != CUOPT_SUCCESS) goto DONE; + status = cuOptSetLogLevel(settings, CUOPT_LOG_LEVEL_OFF); + if (status != CUOPT_SUCCESS) goto DONE; + + status = cuOptSolve(problem, settings, &solution); + if (status != CUOPT_SUCCESS) goto DONE; + + if (ctx.calls != 0) { + printf("Expected 0 log calls with LOG_LEVEL_OFF; got %d\n", ctx.calls); + status = CUOPT_INVALID_ARGUMENT; + goto DONE; + } + +DONE: + cuOptDestroyProblem(&problem); + cuOptDestroySolverSettings(&settings); + cuOptDestroySolution(&solution); + return status; +} + +cuopt_int_t test_log_level_invalid(void) +{ + cuOptSolverSettings settings = NULL; + cuopt_int_t status = cuOptCreateSolverSettings(&settings); + if (status != CUOPT_SUCCESS) goto DONE; + + if (cuOptSetLogLevel(settings, -1) != CUOPT_INVALID_ARGUMENT) { + status = CUOPT_INVALID_ARGUMENT; + goto DONE; + } + if (cuOptSetLogLevel(settings, CUOPT_LOG_LEVEL_OFF + 1) != CUOPT_INVALID_ARGUMENT) { + status = CUOPT_INVALID_ARGUMENT; + goto DONE; + } + status = CUOPT_SUCCESS; + +DONE: + cuOptDestroySolverSettings(&settings); + return status; +} + cuopt_int_t burglar_problem() { cuOptOptimizationProblem problem = NULL; diff --git a/cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp b/cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp index ed0e017cae..54a031646a 100644 --- a/cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp +++ b/cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp @@ -114,6 +114,14 @@ TEST(c_api, mip_get_callbacks_only) { EXPECT_EQ(test_mip_get_callbacks_only(), C TEST(c_api, mip_get_set_callbacks) { EXPECT_EQ(test_mip_get_set_callbacks(), CUOPT_SUCCESS); } +TEST(c_api, log_callback) { EXPECT_EQ(test_log_callback(), CUOPT_SUCCESS); } + +TEST(c_api, log_callback_cleared) { EXPECT_EQ(test_log_callback_cleared(), CUOPT_SUCCESS); } + +TEST(c_api, log_level_off) { EXPECT_EQ(test_log_level_off(), CUOPT_SUCCESS); } + +TEST(c_api, log_level_invalid) { EXPECT_EQ(test_log_level_invalid(), CUOPT_SUCCESS); } + TEST(c_api, burglar) { EXPECT_EQ(burglar_problem(), CUOPT_SUCCESS); } TEST(c_api, test_missing_file) { EXPECT_EQ(test_missing_file(), CUOPT_MPS_FILE_ERROR); } diff --git a/cpp/tests/linear_programming/c_api_tests/c_api_tests.h b/cpp/tests/linear_programming/c_api_tests/c_api_tests.h index a8c3a1f4e4..b754fb0147 100644 --- a/cpp/tests/linear_programming/c_api_tests/c_api_tests.h +++ b/cpp/tests/linear_programming/c_api_tests/c_api_tests.h @@ -30,6 +30,10 @@ cuopt_int_t test_infeasible_problem(); cuopt_int_t test_bad_parameter_name(); cuopt_int_t test_mip_get_callbacks_only(); cuopt_int_t test_mip_get_set_callbacks(); +cuopt_int_t test_log_callback(); +cuopt_int_t test_log_callback_cleared(); +cuopt_int_t test_log_level_off(); +cuopt_int_t test_log_level_invalid(); cuopt_int_t test_ranged_problem(cuopt_int_t* termination_status_ptr, cuopt_float_t* objective_ptr); cuopt_int_t test_semi_continuous_problem(cuopt_int_t* termination_status_ptr, cuopt_float_t* objective_ptr, From adc2f812542e63ad10f2dae162892e5d94efad51 Mon Sep 17 00:00:00 2001 From: Ramakrishna Prabhu Date: Thu, 30 Jul 2026 10:30:00 -0500 Subject: [PATCH 3/3] refactor(c-api): drop cuOptSetLogLevel, control log level via env var Per review (chris-maes): log levels are developer/debug-facing and should not be part of the public C API. Remove cuOptSetLogLevel and the public CUOPT_LOG_LEVEL_* constants; keep cuOptSetLogCallback. Verbosity is now controlled at runtime by the CUOPT_LOG_LEVEL environment variable (TRACE..OFF, case-insensitive), read in default_level(). - Point two mip_heuristics #if guards at RAPIDS_LOGGER_LOG_LEVEL_* instead of the removed CUOPT_LOG_LEVEL_*, fixing a pre-existing latent bug where those undefined macros evaluated to 0. - Replace the log-level unit tests with a cross-solve callback-lifecycle regression (CodeRabbit). Co-Authored-By: Claude Opus 4.8 (1M context) Signed-off-by: Ramakrishna Prabhu --- .../mathematical_optimization/constants.h | 9 --- .../cuopt/mathematical_optimization/cuopt_c.h | 14 +--- .../diversity/diversity_manager.cu | 2 +- .../feasibility_jump/feasibility_jump.cu | 2 +- cpp/src/pdlp/cuopt_c.cpp | 28 ++------ cpp/src/utilities/logger.cpp | 61 +++++++++++------ cpp/src/utilities/logger.hpp | 6 -- .../c_api_tests/c_api_test.c | 65 +++++++++---------- .../c_api_tests/c_api_tests.cpp | 7 +- .../c_api_tests/c_api_tests.h | 3 +- 10 files changed, 86 insertions(+), 111 deletions(-) diff --git a/cpp/include/cuopt/mathematical_optimization/constants.h b/cpp/include/cuopt/mathematical_optimization/constants.h index d2ba9fa38c..9592389dea 100644 --- a/cpp/include/cuopt/mathematical_optimization/constants.h +++ b/cpp/include/cuopt/mathematical_optimization/constants.h @@ -146,15 +146,6 @@ /* @brief QCQP (barrier) scaling hyper-parameters */ #define CUOPT_QCQP_HYPER_RUIZ_EQUILIBRATION "qcqp_hyper_ruiz_equilibration" -/* @brief Log level constants for cuOptSetLogLevel */ -#define CUOPT_LOG_LEVEL_TRACE 0 -#define CUOPT_LOG_LEVEL_DEBUG 1 -#define CUOPT_LOG_LEVEL_INFO 2 -#define CUOPT_LOG_LEVEL_WARN 3 -#define CUOPT_LOG_LEVEL_ERROR 4 -#define CUOPT_LOG_LEVEL_CRITICAL 5 -#define CUOPT_LOG_LEVEL_OFF 6 - /* @brief MIP determinism mode constants */ #define CUOPT_MODE_OPPORTUNISTIC 0 #define CUOPT_MODE_DETERMINISTIC 1 diff --git a/cpp/include/cuopt/mathematical_optimization/cuopt_c.h b/cpp/include/cuopt/mathematical_optimization/cuopt_c.h index 32628d63c9..ffc296072c 100644 --- a/cpp/include/cuopt/mathematical_optimization/cuopt_c.h +++ b/cpp/include/cuopt/mathematical_optimization/cuopt_c.h @@ -826,7 +826,9 @@ cuopt_int_t cuOptGetFloatParameter(cuOptSolverSettings settings, /** * @brief Type of callback invoked once per log line emitted by the solver. * - * @param level Log level (one of CUOPT_LOG_LEVEL_*). + * @param level Severity of the log line, increasing with value + * (0=trace, 1=debug, 2=info, 3=warn, 4=error, 5=critical). Intended only for + * display/filtering of output, not as a stable programmatic API. * @param message Null-terminated log line without trailing newline. * @param user_data Opaque pointer passed to cuOptSetLogCallback. * @@ -856,16 +858,6 @@ cuopt_int_t cuOptSetLogCallback(cuOptSolverSettings settings, cuOptLogCallback callback, void* user_data); -/** - * @brief Set the solver log verbosity level. - * - * @param[in] settings The solver settings object. - * @param[in] level One of CUOPT_LOG_LEVEL_TRACE … CUOPT_LOG_LEVEL_OFF. - * - * @return A status code indicating success or failure. - */ -cuopt_int_t cuOptSetLogLevel(cuOptSolverSettings settings, int level); - /** * @brief Type of callback for receiving incumbent MIP solutions with user context. * diff --git a/cpp/src/mip_heuristics/diversity/diversity_manager.cu b/cpp/src/mip_heuristics/diversity/diversity_manager.cu index 61c90944f4..96b53c5ba9 100644 --- a/cpp/src/mip_heuristics/diversity/diversity_manager.cu +++ b/cpp/src/mip_heuristics/diversity/diversity_manager.cu @@ -207,7 +207,7 @@ void diversity_manager_t::add_user_given_solutions( *problem_ptr->original_problem_ptr, h_original, h_crushed); init_sol_assignment = cuopt::device_copy(h_crushed, sol.handle_ptr->get_stream()); -#if CUOPT_LOG_ACTIVE_LEVEL <= CUOPT_LOG_LEVEL_DEBUG +#if CUOPT_LOG_ACTIVE_LEVEL <= RAPIDS_LOGGER_LOG_LEVEL_DEBUG const auto& reduced_problem = *problem_ptr->original_problem_ptr; const std::vector h_red_obj = reduced_problem.get_objective_coefficients_host(); const std::vector& h_ori_obj = presolver_ptr->get_original_objective_coefficients(); diff --git a/cpp/src/mip_heuristics/feasibility_jump/feasibility_jump.cu b/cpp/src/mip_heuristics/feasibility_jump/feasibility_jump.cu index a6665e57e1..44fd71cb92 100644 --- a/cpp/src/mip_heuristics/feasibility_jump/feasibility_jump.cu +++ b/cpp/src/mip_heuristics/feasibility_jump/feasibility_jump.cu @@ -945,7 +945,7 @@ i_t fj_t::host_loop(solution_t& solution, i_t climber_idx) } } } -#if CUOPT_LOG_ACTIVE_LEVEL == CUOPT_LOG_LEVEL_TRACE +#if CUOPT_LOG_ACTIVE_LEVEL == RAPIDS_LOGGER_LOG_LEVEL_TRACE auto h_sol = cuopt::host_copy(solution.assignment, climber_stream); static std::set> solutions_set; bool same_sol = solutions_set.count(h_sol) > 0; diff --git a/cpp/src/pdlp/cuopt_c.cpp b/cpp/src/pdlp/cuopt_c.cpp index c7556d2085..7de24deb13 100644 --- a/cpp/src/pdlp/cuopt_c.cpp +++ b/cpp/src/pdlp/cuopt_c.cpp @@ -92,8 +92,6 @@ struct solver_settings_handle_t { // Log callback registered via cuOptSetLogCallback cuOptLogCallback log_callback{nullptr}; void* log_callback_user_data{nullptr}; - // Log level override registered via cuOptSetLogLevel (-1 = use default) - int log_level{-1}; }; solver_settings_handle_t* get_settings_handle(cuOptSolverSettings settings) @@ -1075,8 +1073,8 @@ cuopt_int_t cuOptSetMIPSetSolutionCallback(cuOptSolverSettings settings, } cuopt_int_t cuOptSetLogCallback(cuOptSolverSettings settings, - cuOptLogCallback callback, - void* user_data) + cuOptLogCallback callback, + void* user_data) { if (settings == nullptr) { return CUOPT_INVALID_ARGUMENT; } solver_settings_handle_t* handle = get_settings_handle(settings); @@ -1085,16 +1083,6 @@ cuopt_int_t cuOptSetLogCallback(cuOptSolverSettings settings, return CUOPT_SUCCESS; } -cuopt_int_t cuOptSetLogLevel(cuOptSolverSettings settings, int level) -{ - if (settings == nullptr) { return CUOPT_INVALID_ARGUMENT; } - if (level < CUOPT_LOG_LEVEL_TRACE || level > CUOPT_LOG_LEVEL_OFF) { - return CUOPT_INVALID_ARGUMENT; - } - get_settings_handle(settings)->log_level = level; - return CUOPT_SUCCESS; -} - cuopt_int_t cuOptSetInitialPrimalSolution(cuOptSolverSettings settings, const cuopt_float_t* primal_solution, cuopt_int_t num_variables) @@ -1171,28 +1159,22 @@ cuopt_int_t cuOptSolve(cuOptOptimizationProblem problem, if (settings == nullptr) { return CUOPT_INVALID_ARGUMENT; } if (solution_ptr == nullptr) { return CUOPT_INVALID_ARGUMENT; } - // Install user log callback / level so init_logger_t inside the solver picks them up. - // The RAII guard clears them on scope exit (whether by return or exception). + // Install user log callback so init_logger_t inside the solver picks it up. + // The RAII guard clears it on scope exit (whether by return or exception). solver_settings_handle_t* handle = get_settings_handle(settings); struct log_scope_guard_t { bool has_callback; - bool has_level; ~log_scope_guard_t() { if (has_callback) { cuopt::clear_pending_log_callback(); } - if (has_level) { cuopt::clear_pending_log_level(); } } - } log_scope{false, false}; + } log_scope{false}; if (handle->log_callback) { // cuOptLogCallback and log_callback_with_data_t share the same signature. cuopt::set_pending_log_callback(handle->log_callback, handle->log_callback_user_data); log_scope.has_callback = true; } - if (handle->log_level >= 0) { - cuopt::set_pending_log_level(handle->log_level); - log_scope.has_level = true; - } problem_and_stream_view_t* problem_and_stream_view = static_cast(problem); diff --git a/cpp/src/utilities/logger.cpp b/cpp/src/utilities/logger.cpp index 81e7275c0e..8251da7941 100644 --- a/cpp/src/utilities/logger.cpp +++ b/cpp/src/utilities/logger.cpp @@ -1,6 +1,6 @@ /* clang-format off */ /* - * SPDX-FileCopyrightText: Copyright (c) 2025, NVIDIA CORPORATION & AFFILIATES. All rights reserved. + * SPDX-FileCopyrightText: Copyright (c) 2025-2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved. * SPDX-License-Identifier: Apache-2.0 */ /* clang-format on */ @@ -8,6 +8,11 @@ #include #include +#include +#include +#include +#include + namespace cuopt { struct buffered_entry { @@ -82,13 +87,44 @@ rapids_logger::sink_ptr default_sink() */ inline std::string default_pattern() { return "[%Y-%m-%d %H:%M:%S:%f] [%n] [%-6l] %v"; } +/** + * @brief Runtime log-level override from the `CUOPT_LOG_LEVEL` environment variable. + * + * Accepts a level name (case-insensitive): TRACE, DEBUG, INFO, WARN, ERROR, CRITICAL, OFF. + * Returns std::nullopt if the variable is unset or holds an unrecognised value. + * + * @note Statements below the compile-time `CUOPT_LOG_ACTIVE_LEVEL` (default INFO) are + * removed at build time, so raising verbosity above the build level has no effect; + * lowering it (e.g. WARN/ERROR/OFF to suppress output) always works. + */ +inline std::optional env_log_level() +{ + const char* env = std::getenv("CUOPT_LOG_LEVEL"); + if (env == nullptr) { return std::nullopt; } + std::string level{env}; + std::transform(level.begin(), level.end(), level.begin(), [](unsigned char c) { + return static_cast(std::toupper(c)); + }); + if (level == "TRACE") { return rapids_logger::level_enum::trace; } + if (level == "DEBUG") { return rapids_logger::level_enum::debug; } + if (level == "INFO") { return rapids_logger::level_enum::info; } + if (level == "WARN") { return rapids_logger::level_enum::warn; } + if (level == "ERROR") { return rapids_logger::level_enum::error; } + if (level == "CRITICAL") { return rapids_logger::level_enum::critical; } + if (level == "OFF") { return rapids_logger::level_enum::off; } + return std::nullopt; // unrecognised value: keep the compiled default +} + /** * @brief Returns the default log level for the global logger. * + * The `CUOPT_LOG_LEVEL` environment variable, when set, overrides the compile-time default. + * * @return rapids_logger::level_enum The default log level. */ inline rapids_logger::level_enum default_level() { + if (auto lvl = env_log_level()) { return *lvl; } #if CUOPT_LOG_ACTIVE_LEVEL == RAPIDS_LOGGER_LOG_LEVEL_TRACE return rapids_logger::level_enum::trace; #elif CUOPT_LOG_ACTIVE_LEVEL == RAPIDS_LOGGER_LOG_LEVEL_DEBUG @@ -167,11 +203,10 @@ static std::weak_ptr g_active_guard; // while the sink is alive, and the sink is removed (in reset_default_logger) before // this pointer is cleared. -// Pending user log callback/level set by the C API before cuOptSolve. +// Pending user log callback set by the C API before cuOptSolve. // Consumed once (under g_guard_mutex) by init_logger_t to build the guard state. static log_callback_with_data_t g_pending_callback = nullptr; static void* g_pending_callback_data = nullptr; -static int g_pending_log_level = -1; // -1 = use compiled default static void user_log_bridge(int lvl, const char* msg) { @@ -195,18 +230,6 @@ void clear_pending_log_callback() g_pending_callback_data = nullptr; } -void set_pending_log_level(int level) -{ - std::lock_guard lock(g_guard_mutex); - g_pending_log_level = level; -} - -void clear_pending_log_level() -{ - std::lock_guard lock(g_guard_mutex); - g_pending_log_level = -1; -} - init_logger_t::init_logger_t(std::string log_file, bool log_to_console) { std::lock_guard lock(g_guard_mutex); @@ -233,8 +256,8 @@ init_logger_t::init_logger_t(std::string log_file, bool log_to_console) // Capture pending callback into the guard so the bridge reads stable (immutable) state. auto guard = std::make_shared(); if (g_pending_callback) { - guard->callback_state = - std::make_unique(captured_log_callback_t{g_pending_callback, g_pending_callback_data}); + guard->callback_state = std::make_unique( + captured_log_callback_t{g_pending_callback, g_pending_callback_data}); g_active_log_callback = guard->callback_state.get(); cuopt::default_logger().sinks().push_back( std::make_shared(user_log_bridge)); @@ -246,10 +269,6 @@ init_logger_t::init_logger_t(std::string log_file, bool log_to_console) cuopt::default_logger().set_pattern(cuopt::default_pattern()); #endif - if (g_pending_log_level >= 0) { - cuopt::default_logger().set_level(static_cast(g_pending_log_level)); - } - // Extract messages from the global buffer and log to the default logger auto buffered_messages = global_log_buffer().drain_all(); for (const auto& entry : buffered_messages) { diff --git a/cpp/src/utilities/logger.hpp b/cpp/src/utilities/logger.hpp index e7ece939b9..8947e6aa76 100644 --- a/cpp/src/utilities/logger.hpp +++ b/cpp/src/utilities/logger.hpp @@ -50,12 +50,6 @@ using log_callback_with_data_t = void (*)(int level, const char* message, void* void set_pending_log_callback(log_callback_with_data_t cb, void* user_data); void clear_pending_log_callback(); -/** - * @brief Override the log level for the next init_logger_t. Pass -1 to restore the default. - */ -void set_pending_log_level(int level); -void clear_pending_log_level(); - // Ref-counted logger initializer class init_logger_t { // Using shared_ptr for ref-counting diff --git a/cpp/tests/linear_programming/c_api_tests/c_api_test.c b/cpp/tests/linear_programming/c_api_tests/c_api_test.c index 8c77016835..5acc1ebc69 100644 --- a/cpp/tests/linear_programming/c_api_tests/c_api_test.c +++ b/cpp/tests/linear_programming/c_api_tests/c_api_test.c @@ -396,54 +396,51 @@ cuopt_int_t test_log_callback_cleared(void) return status; } -cuopt_int_t test_log_level_off(void) +/* A callback registered for one solve must not fire on a later solve that uses + * fresh settings with no callback — otherwise a stale callback could run against + * destroyed user_data once the first solve's RAII scope ends. */ +cuopt_int_t test_log_callback_not_leaked_across_solves(void) { - cuOptOptimizationProblem problem = NULL; - cuOptSolverSettings settings = NULL; - cuOptSolution solution = NULL; - log_cb_context_t ctx = {0, NULL}; - cuopt_int_t status = make_trivial_lp(&problem, &settings); + cuOptOptimizationProblem problem1 = NULL; + cuOptSolverSettings settings1 = NULL; + cuOptSolution solution1 = NULL; + cuOptOptimizationProblem problem2 = NULL; + cuOptSolverSettings settings2 = NULL; + cuOptSolution solution2 = NULL; + log_cb_context_t ctx = {0, NULL}; + int calls_after_first = 0; + + cuopt_int_t status = make_trivial_lp(&problem1, &settings1); if (status != CUOPT_SUCCESS) goto DONE; - status = cuOptSetLogCallback(settings, counting_log_callback, &ctx); - if (status != CUOPT_SUCCESS) goto DONE; - status = cuOptSetLogLevel(settings, CUOPT_LOG_LEVEL_OFF); + status = cuOptSetLogCallback(settings1, counting_log_callback, &ctx); if (status != CUOPT_SUCCESS) goto DONE; - - status = cuOptSolve(problem, settings, &solution); + status = cuOptSolve(problem1, settings1, &solution1); if (status != CUOPT_SUCCESS) goto DONE; - if (ctx.calls != 0) { - printf("Expected 0 log calls with LOG_LEVEL_OFF; got %d\n", ctx.calls); - status = CUOPT_INVALID_ARGUMENT; - goto DONE; - } - -DONE: - cuOptDestroyProblem(&problem); - cuOptDestroySolverSettings(&settings); - cuOptDestroySolution(&solution); - return status; -} + calls_after_first = ctx.calls; -cuopt_int_t test_log_level_invalid(void) -{ - cuOptSolverSettings settings = NULL; - cuopt_int_t status = cuOptCreateSolverSettings(&settings); + /* Second solve uses fresh settings with no callback registered. */ + status = make_trivial_lp(&problem2, &settings2); + if (status != CUOPT_SUCCESS) goto DONE; + status = cuOptSolve(problem2, settings2, &solution2); if (status != CUOPT_SUCCESS) goto DONE; - if (cuOptSetLogLevel(settings, -1) != CUOPT_INVALID_ARGUMENT) { + if (ctx.calls != calls_after_first) { + printf("Callback leaked across solves; expected %d calls, got %d\n", + calls_after_first, + ctx.calls); status = CUOPT_INVALID_ARGUMENT; goto DONE; } - if (cuOptSetLogLevel(settings, CUOPT_LOG_LEVEL_OFF + 1) != CUOPT_INVALID_ARGUMENT) { - status = CUOPT_INVALID_ARGUMENT; - goto DONE; - } - status = CUOPT_SUCCESS; DONE: - cuOptDestroySolverSettings(&settings); + cuOptDestroyProblem(&problem1); + cuOptDestroySolverSettings(&settings1); + cuOptDestroySolution(&solution1); + cuOptDestroyProblem(&problem2); + cuOptDestroySolverSettings(&settings2); + cuOptDestroySolution(&solution2); return status; } diff --git a/cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp b/cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp index 54a031646a..3575865709 100644 --- a/cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp +++ b/cpp/tests/linear_programming/c_api_tests/c_api_tests.cpp @@ -118,9 +118,10 @@ TEST(c_api, log_callback) { EXPECT_EQ(test_log_callback(), CUOPT_SUCCESS); } TEST(c_api, log_callback_cleared) { EXPECT_EQ(test_log_callback_cleared(), CUOPT_SUCCESS); } -TEST(c_api, log_level_off) { EXPECT_EQ(test_log_level_off(), CUOPT_SUCCESS); } - -TEST(c_api, log_level_invalid) { EXPECT_EQ(test_log_level_invalid(), CUOPT_SUCCESS); } +TEST(c_api, log_callback_not_leaked_across_solves) +{ + EXPECT_EQ(test_log_callback_not_leaked_across_solves(), CUOPT_SUCCESS); +} TEST(c_api, burglar) { EXPECT_EQ(burglar_problem(), CUOPT_SUCCESS); } diff --git a/cpp/tests/linear_programming/c_api_tests/c_api_tests.h b/cpp/tests/linear_programming/c_api_tests/c_api_tests.h index b754fb0147..5cde8c3915 100644 --- a/cpp/tests/linear_programming/c_api_tests/c_api_tests.h +++ b/cpp/tests/linear_programming/c_api_tests/c_api_tests.h @@ -32,8 +32,7 @@ cuopt_int_t test_mip_get_callbacks_only(); cuopt_int_t test_mip_get_set_callbacks(); cuopt_int_t test_log_callback(); cuopt_int_t test_log_callback_cleared(); -cuopt_int_t test_log_level_off(); -cuopt_int_t test_log_level_invalid(); +cuopt_int_t test_log_callback_not_leaked_across_solves(); cuopt_int_t test_ranged_problem(cuopt_int_t* termination_status_ptr, cuopt_float_t* objective_ptr); cuopt_int_t test_semi_continuous_problem(cuopt_int_t* termination_status_ptr, cuopt_float_t* objective_ptr,