-
Notifications
You must be signed in to change notification settings - Fork 18
Update verbose logging mechanism to enable callback provided by user #67
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
Conversation
sync changes
latest changes
…-logic Expose detector order creation
Make simplex init_ilp private and remove binding
…ract-build Add CMake build system
…d-simplexconfig-for-verbose-callb feat: add verbose callback for decoders
|
not sure why the CI checks are failing 😕 it seems like they are getting cut off halfway |
|
did you run the clang formatter on the code locally? ... the GitHub clang formatter can't parse the code |
Thanks, ran it! |
LalehB
left a comment
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.
LGTM!
Thanks Noah!
| if (config.verbose) { | ||
| std::cout << "t0 = " << t0 << " t1 = " << t1 << std::endl; | ||
| } | ||
| config.log_stream << "t0 = " << t0 << " t1 = " << t1 << std::endl; |
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.
should we also have this: if (config.log_stream.active) { for this log_stream?
NoureldinYosri
left a comment
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.
@noajshu do we want the callback solution or do you want to wait until I figure out the verbose thing?
| @@ -0,0 +1,101 @@ | |||
| cmake_minimum_required(VERSION 3.16) | |||
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.
this change is unrelated to the functionality in the PR title
|
Abandoning this because #68 fixes the problem |
Refactored verbose logging mechanism to use a
CallbackStreamclass that can be given a callback handlerSince AI agents have trouble with Bazel, I added a CMake build system.
verbose_callback=None(the default) in the config ctor, it will be a no-op and all verbose logging will be disabled.