Changed callback support to use C++ rather than C for increased memory safety and ownership. e.g., avoid out-of-bounds, especially after changes to the model.

Includes a small breaking change for the C API: renamed HighsCallbackDataOut to be Highs*C*CallbackDataOut.

Similar with HighsCCallbackDataIn, but it also requires user_solution_size to be set.  We will only access elements in memory up to user_solution_size, however we won't do anything unless user_solution_size == num_cols.

Also added minor usability changes to highspy callbacks + a setObjective function that should've been previously added.

Minor breaking change to highspy, the mip_solution no longer needs/supports to_array(n).

Author: mathgeekcoder

check/TestCAPI.c

@@ -12,7 +12,7 @@
 const HighsInt dev_run = 0;
 const double double_equal_tolerance = 1e-5;
 
-void checkGetCallbackDataOutPointer(const HighsCallbackDataOut* data_out,
+void checkGetCallbackDataOutPointer(const HighsCCallbackDataOut* data_out,
                                     const char* name, HighsInt valid) {
   const void* name_p = Highs_getCallbackDataOutItem(data_out, name);
   if (valid) {
@@ -28,7 +28,7 @@ void checkGetCallbackDataOutPointer(const HighsCallbackDataOut* data_out,
   }
 }
 
-void checkGetCallbackDataOutHighsInt(const HighsCallbackDataOut* data_out,
+void checkGetCallbackDataOutHighsInt(const HighsCCallbackDataOut* data_out,
                                      const char* name, HighsInt value) {
   const void* name_p = Highs_getCallbackDataOutItem(data_out, name);
   if (!name_p) {
@@ -46,7 +46,7 @@ void checkGetCallbackDataOutHighsInt(const HighsCallbackDataOut* data_out,
   }
 }
 
-void checkGetCallbackDataOutInt(const HighsCallbackDataOut* data_out,
+void checkGetCallbackDataOutInt(const HighsCCallbackDataOut* data_out,
                                 const char* name, int value) {
   const void* name_p = Highs_getCallbackDataOutItem(data_out, name);
   if (!name_p) {
@@ -64,7 +64,7 @@ void checkGetCallbackDataOutInt(const HighsCallbackDataOut* data_out,
   }
 }
 
-void checkGetCallbackDataOutInt64(const HighsCallbackDataOut* data_out,
+void checkGetCallbackDataOutInt64(const HighsCCallbackDataOut* data_out,
                                   const char* name, int64_t value) {
   const void* name_p = Highs_getCallbackDataOutItem(data_out, name);
   if (!name_p) {
@@ -82,7 +82,7 @@ void checkGetCallbackDataOutInt64(const HighsCallbackDataOut* data_out,
   }
 }
 
-void checkGetCallbackDataOutDouble(const HighsCallbackDataOut* data_out,
+void checkGetCallbackDataOutDouble(const HighsCCallbackDataOut* data_out,
                                    const char* name, double value) {
   const void* name_p = Highs_getCallbackDataOutItem(data_out, name);
   if (!name_p) {
@@ -101,8 +101,8 @@ void checkGetCallbackDataOutDouble(const HighsCallbackDataOut* data_out,
 }
 
 static void userCallback(const int callback_type, const char* message,
-                         const HighsCallbackDataOut* data_out,
-                         HighsCallbackDataIn* data_in,
+                         const HighsCCallbackDataOut* data_out,
+                         HighsCCallbackDataIn* data_in,
                          void* user_callback_data) {
   // Extract the double value pointed to from void* user_callback_data
   const double local_callback_data =

src/highs_bindings.cpp

@@ -33,7 +33,7 @@ class readonly_ptr_wrapper {
 
   py::array_t<T, py::array::c_style> to_array(std::size_t size) {
     return py::array_t<T, py::array::c_style>(py::buffer_info(
-        ptr, sizeof(T), py::format_descriptor<T>::format(), 1, {size}, {1}));
+        ptr, sizeof(T), py::format_descriptor<T>::format(), 1, {size}, { sizeof(double) }));
   }
 
  private:
@@ -1585,12 +1585,8 @@ PYBIND11_MODULE(_core, m, py::mod_gil_not_used()) {
       .value("kNumCallbackType", HighsCallbackType::kNumCallbackType)
       .export_values();
   // Classes
-  py::class_<readonly_ptr_wrapper<double>>(m, "readonly_ptr_wrapper_double", py::module_local())
-      .def(py::init<double*>())
-      .def("__getitem__", &readonly_ptr_wrapper<double>::operator[])
-      .def("__bool__", &readonly_ptr_wrapper<double>::is_valid)
-      .def("to_array", &readonly_ptr_wrapper<double>::to_array);
-  py::class_<HighsCallbackDataOut>(callbacks, "HighsCallbackDataOut", py::module_local())
+  py::class_<HighsCallbackDataOut>(callbacks, "HighsCallbackDataOut",
+                                   py::module_local())
       .def(py::init<>())
       .def_readwrite("log_type", &HighsCallbackDataOut::log_type)
       .def_readwrite("running_time", &HighsCallbackDataOut::running_time)
@@ -1607,12 +1603,15 @@ PYBIND11_MODULE(_core, m, py::mod_gil_not_used()) {
                      &HighsCallbackDataOut::mip_primal_bound)
       .def_readwrite("mip_dual_bound", &HighsCallbackDataOut::mip_dual_bound)
       .def_readwrite("mip_gap", &HighsCallbackDataOut::mip_gap)
-      .def_property_readonly(
-          "mip_solution",
-          [](const HighsCallbackDataOut& self) -> readonly_ptr_wrapper<double> {
-            return readonly_ptr_wrapper<double>(self.mip_solution);
-          });
+      .def_readwrite("mip_solution", &HighsCallbackDataOut::mip_solution)
+      .def_readwrite("cutpool_num_col", &HighsCallbackDataOut::cutpool_num_col)
+      .def_readwrite("cutpool_index", &HighsCallbackDataOut::cutpool_index)
+      .def_readwrite("cutpool_value", &HighsCallbackDataOut::cutpool_value)
+      .def_readwrite("cutpool_lower", &HighsCallbackDataOut::cutpool_lower)
+      .def_readwrite("cutpool_upper", &HighsCallbackDataOut::cutpool_upper);
+
   py::class_<HighsCallbackDataIn>(callbacks, "HighsCallbackDataIn", py::module_local())
       .def(py::init<>())
-      .def_readwrite("user_interrupt", &HighsCallbackDataIn::user_interrupt);
+      .def_readwrite("user_interrupt", &HighsCallbackDataIn::user_interrupt)
+      .def_readwrite("user_solution", &HighsCallbackDataIn::user_solution);
 }

src/highspy/_core/cb.pyi

@@ -26,6 +26,7 @@ __all__ = [
 
 class HighsCallbackDataIn:
     user_interrupt: int
+    user_solution: list[float]
     def __init__(self) -> None: ...
 
 class HighsCallbackDataOut:
@@ -39,9 +40,14 @@ class HighsCallbackDataOut:
     pdlp_iteration_count: int
     running_time: float
     simplex_iteration_count: int
+    mip_solution: list[float]
+    cutpool_num_col: int
+    cutpool_start: list[int]
+    cutpool_index: list[int]
+    cutpool_value: list[float]
+    cutpool_lower: list[float]
+    cutpool_upper: list[float]
     def __init__(self) -> None: ...
-    @property
-    def mip_solution(self) -> highspy._core.readonly_ptr_wrapper_double: ...
 
 class HighsCallbackType:
     """

src/highspy/highs.py

@@ -13,7 +13,6 @@
     HighsLinearObjective,
     cb,  # type: ignore
     _Highs,  # type: ignore
-    readonly_ptr_wrapper_double,
     kHighsInf,
 )
 
@@ -179,19 +178,17 @@ def optimize(self):
         """
         return self.solve()
 
-    # reset the objective and sense, then solve
-    def minimize(self, obj: Optional[Union[highs_var, highs_linear_expression]] = None):
+    # reset the objective
+    def setObjective(self, obj: Optional[Union[highs_var, highs_linear_expression]] = None, sense: Optional[ObjSense] = None):
         """
-        Solves a minimization of the objective and optionally updates the costs.
+        Updates the costs.
 
         Args:
             obj: An optional highs_linear_expression representing the new objective function.
+            sense: An optional ObjSense value representing the new objective sense.
 
         Raises:
             Exception: If obj is an inequality or not a highs_linear_expression.
-
-        Returns:
-            A HighsStatus object containing the solve status after minimization.
         """
         if obj is not None:
             # if we have a single variable, wrap it in a linear expression
@@ -212,13 +209,13 @@ def minimize(self, obj: Optional[Union[highs_var, highs_linear_expression]] = No
             super().changeColsCost(len(idxs), idxs, vals)
             super().changeObjectiveOffset(expr.constant or 0.0)
 
-        super().changeObjectiveSense(ObjSense.kMinimize)
-        return self.solve()
+        if sense is not None:
+            super().changeObjectiveSense(sense)
 
     # reset the objective and sense, then solve
-    def maximize(self, obj: Optional[Union[highs_var, highs_linear_expression]] = None):
+    def minimize(self, obj: Optional[Union[highs_var, highs_linear_expression]] = None):
         """
-        Solves a maximization of the objective and optionally updates the costs.
+        Solves a minimization of the objective and optionally updates the costs.
 
         Args:
             obj: An optional highs_linear_expression representing the new objective function.
@@ -227,32 +224,31 @@ def maximize(self, obj: Optional[Union[highs_var, highs_linear_expression]] = No
             Exception: If obj is an inequality or not a highs_linear_expression.
 
         Returns:
-            A HighsStatus object containing the solve status after maximization.
+            A HighsStatus object containing the solve status after minimization.
         """
-        if obj is not None:
-            # if we have a single variable, wrap it in a linear expression
-            expr = highs_linear_expression(obj) if isinstance(obj, highs_var) else obj
+        self.setObjective(obj, ObjSense.kMinimize)
+        return self.solve()
 
-            if expr.bounds is not None:
-                raise Exception("Objective cannot be an inequality")
+    # reset the objective and sense, then solve
+    def maximize(self, obj: Optional[Union[highs_var, highs_linear_expression]] = None):
+        """
+        Solves a maximization of the objective and optionally updates the costs.
 
-            # reset objective
-            super().changeColsCost(
-                self.numVariables,
-                np.arange(self.numVariables, dtype=np.int32),
-                np.full(self.numVariables, 0, dtype=np.float64),
-            )
+        Args:
+            obj: An optional highs_linear_expression representing the new objective function.
 
-            # if we have duplicate variables, add the vals
-            idxs, vals = expr.unique_elements()
-            super().changeColsCost(len(idxs), idxs, vals)
-            super().changeObjectiveOffset(expr.constant or 0.0)
+        Raises:
+            Exception: If obj is an inequality or not a highs_linear_expression.
 
-        super().changeObjectiveSense(ObjSense.kMaximize)
+        Returns:
+            A HighsStatus object containing the solve status after maximization.
+        """
+        self.setObjective(obj, ObjSense.kMaximize)
         return self.solve()
+
     @staticmethod
     def internal_get_value(
-        array_values: Union[Sequence[float], np.ndarray[Any, np.dtype[np.float64]], readonly_ptr_wrapper_double],
+        array_values: Union[Sequence[float], np.ndarray[Any, np.dtype[np.float64]]],
         index_collection: Union[
             Integral, highs_var, highs_cons, highs_linear_expression, Mapping[Any, Any], Sequence[Any], np.ndarray[Any, np.dtype[Any]]
         ],
@@ -1203,7 +1199,7 @@ def __internal_callback(
         data_in: Optional[cb.HighsCallbackDataIn],
         user_callback_data: Any,
     ):
-        user_callback_data.callbacks[int(callback_type)].fire(message, data_out, data_in)
+        user_callback_data.callbacks[int(callback_type)].fire(callback_type, message, data_out, data_in)
 
     def enableCallbacks(self):
         """
@@ -1216,9 +1212,16 @@ def enableCallbacks(self):
             if len(c.callbacks) > 0:
                 self.startCallback(c.callback_type)
 
+    def clearCallbacks(self):
+        """
+        Clears all callbacks.
+        """
+        for c in self.callbacks:
+            c.clear()
+
     def disableCallbacks(self):
         """
-        Disables all callbacks.
+        Disables all callbacks, but does not clear them.
         """
         status = super().setCallback(None, None)  # this will also stop all callbacks
 
@@ -1264,8 +1267,8 @@ def HandleUserInterrupt(self, value: bool):
             self.cbMipInterrupt -= self.__user_interrupt_event
 
     def __user_interrupt_event(self, e: HighsCallbackEvent):
-        if self.__solver_should_stop and e.data_in is not None:
-            e.data_in.user_interrupt = True
+        if self.__solver_should_stop:
+            e.interrupt()
 
     @property
     def cbLogging(self):
@@ -1359,20 +1362,29 @@ def cbMipDefineLazyConstraints(self, value: HighsCallback):
 ## Callback support
 ##
 class HighsCallbackEvent(object):
-    __slots__ = ["message", "data_out", "data_in", "user_data"]
+    __slots__ = ["callback_type", "message", "data_out", "data_in", "user_data"]
 
     def __init__(
         self,
+        callback_type: cb.HighsCallbackType,
         message: str,
         data_out: cb.HighsCallbackDataOut,
         data_in: Optional[cb.HighsCallbackDataIn],
-        user_data: Optional[Any],
+        user_data: Optional[Any]
     ):
+        self.callback_type = callback_type
         self.message = message
         self.data_out = data_out
         self.data_in = data_in
         self.user_data = user_data
 
+    def interrupt(self, interrupt_value: bool = True):
+        """
+        Sets the user interrupt flag in the callback data.
+        """
+        if self.data_in is not None:
+            self.data_in.user_interrupt = interrupt_value
+
     def val(
         self,
         var_expr: Union[Integral, highs_var, highs_cons, highs_linear_expression, Mapping[Any, Any], np.ndarray[Any, np.dtype[Any]]],
@@ -1468,14 +1480,15 @@ def clear(self):
 
     def fire(
         self,
+        callback_type: cb.HighsCallbackType,
         message: str,
         data_out: cb.HighsCallbackDataOut,
         data_in: cb.HighsCallbackDataIn,
     ):
         """
         Fires the event, executing all subscribed callbacks.
         """
-        e = HighsCallbackEvent(message, data_out, data_in, None)
+        e = HighsCallbackEvent(callback_type, message, data_out, data_in, None)
 
         for fn, user_data in zip(self.callbacks, self.user_callback_data):
             e.user_data = user_data
@@ -1792,7 +1805,7 @@ def copy(self):
 
     def evaluate(
         self,
-        values: Union[Sequence[float], np.ndarray[Any, np.dtype[np.float64]], readonly_ptr_wrapper_double],
+        values: Union[Sequence[float], np.ndarray[Any, np.dtype[np.float64]]],
     ) -> Union[float, bool]:
         """
         Evaluates the linear expression given a solution array (values).

src/interfaces/highs_c_api.cpp

@@ -1433,7 +1433,7 @@ void Highs_resetGlobalScheduler(HighsInt blocking) {
   Highs::resetGlobalScheduler(blocking != 0);
 }
 
-const void* Highs_getCallbackDataOutItem(const HighsCallbackDataOut* data_out,
+const void* Highs_getCallbackDataOutItem(const HighsCCallbackDataOut* data_out,
                                          const char* item_name) {
   // Accessor function for HighsCallbackDataOut
   //

src/interfaces/highs_c_api.h

@@ -2359,7 +2359,7 @@ void Highs_resetGlobalScheduler(const HighsInt blocking);
  * @returns A void* pointer to the callback data item, or NULL if item_name not
  * valid
  */
-const void* Highs_getCallbackDataOutItem(const HighsCallbackDataOut* data_out,
+const void* Highs_getCallbackDataOutItem(const HighsCCallbackDataOut* data_out,
                                          const char* item_name);
 
 // *********************

src/io/HighsIO.cpp

@@ -151,7 +151,7 @@ void highsLogUser(const HighsLogOptions& log_options_, const HighsLogType type,
     if (log_options_.user_callback_active) {
       assert(log_options_.user_callback);
       HighsCallbackDataOut data_out;
-      data_out.log_type = int(type);
+      data_out.log_type = type;
       log_options_.user_callback(kCallbackLogging, msgbuffer.data(), &data_out,
                                  nullptr, log_options_.user_callback_data);
     }
@@ -210,7 +210,7 @@ void highsLogDev(const HighsLogOptions& log_options_, const HighsLogType type,
     } else if (log_options_.user_callback_active) {
       assert(log_options_.user_callback);
       HighsCallbackDataOut data_out;
-      data_out.log_type = int(type);
+      data_out.log_type = type;
       log_options_.user_callback(kCallbackLogging, msgbuffer.data(), &data_out,
                                  nullptr, log_options_.user_callback_data);
     }

src/lp_data/Highs.cpp

@@ -2227,9 +2227,18 @@ HighsStatus Highs::setCallback(HighsCCallbackType c_callback,
                                void* user_callback_data) {
   this->callback_.clear();
   this->callback_.user_callback =
-      [c_callback](int a, const std::string& b, const HighsCallbackDataOut* c,
-                   HighsCallbackDataIn* d,
-                   void* e) { c_callback(a, b.c_str(), c, d, e); };
+      [c_callback](int a, const std::string& b, const HighsCallbackDataOut* cb_out,
+                   HighsCallbackDataIn* cb_in,
+                   void* e) { 
+        HighsCCallbackDataOut cc_out = static_cast<HighsCCallbackDataOut>(*cb_out);
+        HighsCCallbackDataIn cc_in;
+        cc_in.user_interrupt = 0;
+        cc_in.user_solution_size = 0;
+        cc_in.user_solution = nullptr;
+
+        c_callback(a, b.c_str(), &cc_out, &cc_in, e); 
+        *cb_in = cc_in;  // copy the data in
+      };
   this->callback_.user_callback_data = user_callback_data;
 
   options_.log_options.user_callback = this->callback_.user_callback;