Highs::setCallback() does not really work

[psychon]

Hi there,

I am using HiGHS as a library and I am trying to integrate it into the existing logging framework via setCallback(), but the callback is never called unless I also set a log file and enable the output flag (for which I found no documentation).

Reproducer:

diff --git a/check/TestLogging.cpp b/check/TestLogging.cpp
index cdda2cc8d..1c27faa24 100644
--- a/check/TestLogging.cpp
+++ b/check/TestLogging.cpp
@@ -63,3 +63,41 @@ TEST_CASE("no-logging", "[highs_logging]") {
   return_status = highs.run();
   REQUIRE(return_status == HighsStatus::kOk);
 }
+
+// cmake -S . -B build -G Ninja -DBUILD_TESTING=ON -DALL_TESTS=ON
+
+void test_logging_callback(bool setFile) {
+  auto output = std::make_shared<std::string>();
+  HighsStatus return_status;
+
+  std::string model = "adlittle";
+  std::string model_file = std::string(HIGHS_DIR) + "/check/instances/" + model + ".mps";
+
+  Highs highs;
+
+  highs.setOptionValue("output_flag", true);
+  highs.setOptionValue("log_to_console", false);
+  highs.setCallback([output](int, const std::string& message, const HighsCallbackDataOut*, const HighsCallbackDataIn*, void*) {
+      *output += message;
+    });
+  highs.startCallback(kCallbackLogging);
+  if (setFile) {
+    highs.openLogFile("log_file.txt");
+  }
+
+  return_status = highs.readModel(model_file);
+  REQUIRE(return_status == HighsStatus::kOk);
+
+  return_status = highs.run();
+  REQUIRE(return_status == HighsStatus::kOk);
+
+  REQUIRE(*output != "");
+}
+
+TEST_CASE("without file", "[highs_logging]") {
+  test_logging_callback(false);
+}
+
+TEST_CASE("with file", "[highs_logging]") {
+  test_logging_callback(true);
+}

The without file test fails, the with file test passes. The with file test creates a log file, but it is empty.

I am not quite sure how this is supposed to work, but I would suggest something like the following. This makes my new tests pass and the log output also appears in the log file. Note that this change makes unit_tests print lots of Hs to the console, so... something is off, I guess?

diff --git a/src/io/HighsIO.cpp b/src/io/HighsIO.cpp
index 96c990495..05ac26828 100644
--- a/src/io/HighsIO.cpp
+++ b/src/io/HighsIO.cpp
@@ -95,8 +95,14 @@ std::array<char, 32> highsDoubleToString(const double val,
 
 void highsLogUser(const HighsLogOptions& log_options_, const HighsLogType type,
                   const char* format, ...) {
-  if (!*log_options_.output_flag ||
-      (log_options_.log_stream == NULL && !*log_options_.log_to_console))
+  if (!*log_options_.output_flag)
+    return;
+
+  const bool log_stream = log_options_.log_stream != NULL || *log_options_.log_to_console;
+  const bool use_log_callback =
+      log_options_.user_log_callback ||
+      (log_options_.user_callback && log_options_.user_callback_active);
+  if (!log_stream && !use_log_callback)
     return;
   // highsLogUser should not be passed HighsLogType::kDetailed or
   // HighsLogType::kVerbose
@@ -107,11 +113,8 @@ void highsLogUser(const HighsLogOptions& log_options_, const HighsLogType type,
   va_list argptr;
   va_start(argptr, format);
   const bool flush_streams = true;
-  const bool use_log_callback =
-      log_options_.user_log_callback ||
-      (log_options_.user_callback && log_options_.user_callback_active);
 
-  if (!use_log_callback) {
+  if (log_stream) {
     // Write to log file stream unless it is NULL
     if (log_options_.log_stream) {
       if (prefix)
@@ -127,7 +130,8 @@ void highsLogUser(const HighsLogOptions& log_options_, const HighsLogType type,
       vfprintf(stdout, format, argptr);
       if (flush_streams) fflush(stdout);
     }
-  } else {
+  }
+  if (use_log_callback) {
     size_t len = 0;
     std::array<char, kIoBufferSize> msgbuffer = {};
     if (prefix) {

The above was done with current git master, which is v1.9.0-1-gbdf95f2e0

[jajhall]

Thanks for flagging this up. I'll look at it in due course. All the "H" characters you see are due to a unit test for the original logging callback that is deprecated, and activated if log_options_.user_log_callback is true