From ef240d6c0890ca20b96dad5ab1dc2caa08e398f3 Mon Sep 17 00:00:00 2001 From: Duc Le Date: Sun, 24 Sep 2023 18:51:46 +0100 Subject: [PATCH] Fix euphonic/tobyfit errors Change call_python to use global dict Update to work on Python 3.11 Add setdlopenflags(DEEPBIND) for Linux --- CHANGELOG.md | 2 ++ CMakeLists.txt | 2 +- libpymcr/Matlab.py | 12 ++++++++++++ libpymcr/__init__.py | 2 ++ libpymcr/utils.py | 2 +- src/call.m | 2 +- src/call_python.cpp | 13 +++++++------ src/type_converter.cpp | 7 +++++-- src/type_converter.hpp | 1 + 9 files changed, 32 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21d5b98..8925084 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ Bugfixes for PySpinW identified by users and during the RAL-India workshop. * Fix a segfault when temporary numpy arrays are re-used multiple times in definition of SpinW objects * Change behaviour of tuples and lists. Python tuples now always convert to Matlab cells. Nested lists will convert to Matlab numeric arrays if they are consistent in shape and contain only numbers. This allows Python `([1,2,3], [4,5,6])` to convert to Matlab `{[1 2 3] [4 5 6]}` whereas before it would have converted to Matlab `[1 2 3; 4 5 6]`. Python `[[1,2,3], [4,5,6]]` will still convert to Matlab `[1 2 3; 4 5 6]`. * Fix bug where Matlab commands which return zero outputs fail, e.g. `m.axis([0,1,0,1])` due to incorrectly given `nargout` in Matlab.py / call.m +* New `get_nlhs` algorithm to set `nargout` parameters uses `ast` to better parse cases of nested Matlab calls, like `m.eig(m.rand(3))` but this only works if the full command is on one line; and also does not work for the basic interpreter (but should work in Mantid, Jupyter and Spyder). For those cases, the old `dis` algorithm is used, updated to work with Python 3.11 but does not handle nested calls. +* Fix bugs in `call_python`. Changed mechanism back to using a global `dict` as direct memory address was failing in some nested calls. `call_python` now creates its own instance of `type_converter` to avoid a heap memory error when using the Python initialized object from the `libpymcr` instance. Re-add `DEEPBIND` library loading for Linux Lapack/BLAS to avoid conflict with Matlab. # [v0.1.4](https://github.com/pace-neutrons/libpymcr/compare/v0.1.3...v0.1.4) diff --git a/CMakeLists.txt b/CMakeLists.txt index f2e18db..da3c061 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -8,7 +8,7 @@ set(CMAKE_MACOSX_RPATH TRUE) set(CMAKE_CXX_STANDARD 11) set(CXX_STANDARD_REQUIRED 11) -set(MINIMUM_PYBIND11_VERSION 2.10.0) +set(MINIMUM_PYBIND11_VERSION 2.10.1) set(FETCH_PYBIND11_REPO https://github.com/pybind/pybind11) if (PYTHON_EXECUTABLE) diff --git a/libpymcr/Matlab.py b/libpymcr/Matlab.py index cf17f2f..2ba9792 100644 --- a/libpymcr/Matlab.py +++ b/libpymcr/Matlab.py @@ -11,6 +11,18 @@ _global_matlab_ref = None _has_registered_magic = None +# On Linux we need to load the BLAS/LAPACK libraries with the DEEPBIND +# flag so it doesn't conflict with Matlab's BLAS/LAPACK. +# This only works if users `import libpymcr` before they import scipy... +if platform.system() == 'Linux': + old_flags = sys.getdlopenflags() + sys.setdlopenflags(os.RTLD_NOW | os.RTLD_DEEPBIND) + try: + import scipy.linalg + except ImportError: + pass + sys.setdlopenflags(old_flags) + class _MatlabInstance(object): def __init__(self, ctffile, matlab_dir=None, options=None): diff --git a/libpymcr/__init__.py b/libpymcr/__init__.py index 2db6334..387614d 100644 --- a/libpymcr/__init__.py +++ b/libpymcr/__init__.py @@ -4,3 +4,5 @@ from .Matlab import Matlab from .MatlabProxyObject import MatlabProxyObject from . import utils + +_globalFunctionDict = {} diff --git a/libpymcr/utils.py b/libpymcr/utils.py index 5c64a5a..7a3186d 100644 --- a/libpymcr/utils.py +++ b/libpymcr/utils.py @@ -11,7 +11,7 @@ OPERATOR_NAMES = { - "CALL_FUNCTION", "CALL_FUNCTION_VAR", "CALL_FUNCTION_KW", "CALL_FUNCTION_VAR_KW", + "CALL_FUNCTION", "CALL_FUNCTION_VAR", "CALL_FUNCTION_KW", "CALL_FUNCTION_VAR_KW", "CALL", "UNARY_POSITIVE", "UNARY_NEGATIVE", "UNARY_NOT", "UNARY_CONVERT", "UNARY_INVERT", "GET_ITER", "BINARY_POWER", "BINARY_MULTIPLY", "BINARY_DIVIDE", "BINARY_FLOOR_DIVIDE", "BINARY_TRUE_DIVIDE", "BINARY_MODULO", "BINARY_ADD", "BINARY_SUBTRACT", "BINARY_SUBSCR", "BINARY_LSHIFT", diff --git a/src/call.m b/src/call.m index de88bbf..60f69f2 100644 --- a/src/call.m +++ b/src/call.m @@ -47,7 +47,7 @@ function out = unwrap(in_obj) out = in_obj; if isstruct(in_obj) && isfield(in_obj, 'libpymcr_func_ptr') - out = @(varargin) call('_call_python', [in_obj.libpymcr_func_ptr], varargin{:}); + out = @(varargin) call('_call_python', in_obj.libpymcr_func_ptr, varargin{:}); elseif isa(in_obj, 'containers.Map') && in_obj.isKey('wrapped_oldstyle_class') out = in_obj('wrapped_oldstyle_class'); elseif iscell(in_obj) diff --git a/src/call_python.cpp b/src/call_python.cpp index 6ff3f47..10ef7b2 100644 --- a/src/call_python.cpp +++ b/src/call_python.cpp @@ -26,19 +26,20 @@ class MexFunction : public matlab::mex::Function { public: void operator()(ArgumentList outputs, ArgumentList inputs) { matlab::data::ArrayFactory factory; - if (inputs.size() < 1 || inputs[0].getNumberOfElements() != 1 || - inputs[0].getType() != matlab::data::ArrayType::UINT64) { // Matlab only supports 64-bit - throw std::runtime_error("Input must be pointer to a Python function."); + if (inputs.size() < 1 || inputs[0].getType() != matlab::data::ArrayType::CHAR) { + throw std::runtime_error("Input must be reference to a Python function."); } - uintptr_t key = inputs[0][0]; + matlab::data::CharArray key = inputs[0]; PyGILState_STATE gstate = PyGILState_Ensure(); // GIL{ if (_converter == nullptr) { - _converter = new libpymcr::pymat_converter(); + _converter = new libpymcr::pymat_converter(libpymcr::pymat_converter::NumpyConversion::WRAP); } - PyObject* fn_ptr = reinterpret_cast(key); + py::module pyHoraceFn = py::module::import("libpymcr"); + py::dict fnDict = pyHoraceFn.attr("_globalFunctionDict"); + PyObject *fn_ptr = PyDict_GetItemString(fnDict.ptr(), key.toAscii().c_str()); if (PyCallable_Check(fn_ptr)) { PyObject *result; diff --git a/src/type_converter.cpp b/src/type_converter.cpp index 72d11ea..c4a8782 100644 --- a/src/type_converter.cpp +++ b/src/type_converter.cpp @@ -522,9 +522,12 @@ Array pymat_converter::listtuple_to_cell(PyObject *result, matlab::data::ArrayFa matlab::data::Array pymat_converter::wrap_python_function(PyObject *input, matlab::data::ArrayFactory &factory) { // Wraps a Python function so it can be called using a mex function matlab::data::Array rv; - std::uintptr_t addr = reinterpret_cast(input); + std::string addrstr = std::to_string(reinterpret_cast(input)); rv = factory.createStructArray({1, 1}, std::vector({"libpymcr_func_ptr"})); - rv[0][std::string("libpymcr_func_ptr")] = factory.createScalar(addr); + py::module pyHoraceFn = py::module::import("libpymcr"); + py::dict fnDict = pyHoraceFn.attr("_globalFunctionDict"); + PyDict_SetItemString(fnDict.ptr(), addrstr.c_str(), input); + rv[0][std::string("libpymcr_func_ptr")] = factory.createCharArray(addrstr.c_str()); return rv; } diff --git a/src/type_converter.hpp b/src/type_converter.hpp index a3e4488..3f04ac6 100644 --- a/src/type_converter.hpp +++ b/src/type_converter.hpp @@ -16,6 +16,7 @@ using ssize_t = std::ptrdiff_t; #include #include #include +#include #include #define MYINTEGER 1