Member function in highspy has no effect

[BenChampion]

In the context of something like

import highspy as hs
h = hs.Highs()
h.readModel("some_model.mps")
h.run()
sol = getSolution()

doing

sol.row_dual.clear()

has no effect while

sol.row_dual = []

works.

This may also relate to the following discussion.

#2234 (reply in thread)

[mathgeekcoder]

Assuming we want to support this, I think the easiest way is something like:

#include <pybind11/stl_bind.h>

PYBIND11_MAKE_OPAQUE(std::vector<int>)  
PYBIND11_MAKE_OPAQUE(std::vector<double>)  

PYBIND11_MODULE(_core, m, py::mod_gil_not_used()) {
  ...
  py::bind_vector<std::vector<int>>(m, "VectorInt");
  py::bind_vector<std::vector<double>>(m, "VectorDouble");

But then the type of sol.row_dual becomes a VectorDouble instead of a list, which might have other drawbacks.

An alternative is to return numpy arrays instead. They don't have a clear function anyhow, so it's a little closer to our current behaviour. Converting to numpy has been on my TODO list, I wanted to sanity check performance first.

[mathgeekcoder]

FYI: To efficiently return numpy arrays (by reference not value), this is the code:

  py::class_<HighsSolution>(m, "HighsSolution", py::module_local())
      .def(py::init<>())
      ...
      .def_property("row_dual", make_readonly_ptr(&HighsSolution::row_dual),
                    make_setter_ptr(&HighsSolution::row_dual))

There might be nicer ways to do this, e.g., we might be able to have a nicer wrapper function, since we'd need to do this for every vector member. That said, this code:

sol.row_dual.clear()

will now throw an exception, rather than silently failing.

[BenChampion]

@mathgeekcoder

[...]
An alternative is to return numpy arrays instead. They don't have a clear function anyhow, so it's a little closer to our current behaviour. Converting to numpy has been on my TODO list, I wanted to sanity check performance first.
[...]

Thanks Luke!

Pending your performance checks, numpy arrays sound good to me. I don't think there's a specific need for clear(), just for not failing silently. Would this also fix the .[...] indexing silent failures I linked above? (#2234 (reply in thread))

[mathgeekcoder]

Pending your performance checks, numpy arrays sound good to me. I don't think there's a specific need for clear(), just for not failing silently. Would this also fix the .[...] indexing silent failures I linked above? (#2234 (reply in thread))

Absolutely. By default, the lp.integrality_ returns a copy of the array; so lp.integrality_[i] = ... gets a copy, modifies the copy, and throws away the copy.

If we switch to numpy it'll return a reference - so we can update the internals directly. Performance should be faster, but we probably should also check safety when converting everything. BTW: the h.passModel(lp) is needed since h.getLp() also returns a copy; this is okay, since Highs::getLp() is const. It's possible to return a reference to lp, but a lot more effort to enforce read-only.

That said, people should use h.changeColsIntegrality(...) or h.setContinuous(...) / h.setInteger(...) for this particular use case.