-
Notifications
You must be signed in to change notification settings - Fork 223
feat(c-api): add cuOptSetLogCallback #1636
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
3756aaf
a1ce114
adc2f81
dc14e3b
ade6670
351decb
6562dda
46249e7
85cb36d
30632ca
8831f1a
31e6c10
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -828,6 +828,44 @@ cuopt_int_t cuOptGetFloatParameter(cuOptSolverSettings settings, | |
| const char* parameter_name, | ||
| cuopt_float_t* parameter_value); | ||
|
|
||
| /** | ||
| * @brief Type of callback invoked once per standard solver log line. | ||
| * | ||
| * Receives the same lines the solver would print to the console — nothing more. | ||
| * Internal diagnostics (debug and trace messages) are never delivered, and no | ||
| * severity is reported: the callback exists to display or forward solver | ||
| * output, not to let callers classify or branch on it. | ||
| * | ||
| * @param message Null-terminated log line without trailing newline. | ||
| * @param user_data Opaque pointer passed to cuOptSetLogCallback. | ||
| * | ||
| * @note Invoked from the calling thread for a local solve, and from an internal | ||
| * log-streaming thread when the solve runs on a remote server. 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)(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, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How this handled with GRPC?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch — it wasn't, and it is now (30632ca). With
Both remote paths now forward each streamed line to the user callback, and request streaming when a callback is present. The registration is captured at call time rather than read inside the lambda, because Verified against a live and the remote lines are the server's own solver log ( One behavioural note now documented in the header: the callback runs on the calling thread for a local solve, and on the log-streaming thread for a remote one.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you please add a test for this?
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Follow-up: this is now covered by a test as well (31e6c10).
Worth noting why the assertion is what it is: the client emits its own |
||
| cuOptLogCallback callback, | ||
| void* user_data); | ||
|
|
||
| /** | ||
| * @brief Type of callback for receiving incumbent MIP solutions with user context. | ||
| * | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
Repository: NVIDIA/cuopt
Length of output: 50369
🏁 Script executed:
Repository: NVIDIA/cuopt
Length of output: 10753
🏁 Script executed:
Repository: NVIDIA/cuopt
Length of output: 32044
🏁 Script executed:
Repository: NVIDIA/cuopt
Length of output: 928
Document the breaking callback ABI change.
cuOptLogCallbackchanged from three parameters to two. Existing binaries must be rebuilt and update their callback definitions. Confirm the intentional ABI break and add migration guidance to the release documentation.🤖 Prompt for AI Agents
Sources: Coding guidelines, Path instructions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not an ABI break —
cuOptLogCallbackhas never shipped. It is introduced by this PR, so there is no released signature to migrate from:The three-parameter form existed only in this branch's earlier commit (3756aaf), and the two-parameter form (dc14e3b) replaced it before merge. The diff you compared is intra-PR churn, not a change to a public API.
No release-note migration guidance is needed. The signature narrowing was made in response to @chris-maes's review — dropping the level parameter so no severity is exposed through the C API.