Bring Nanobind. Fix object leak in from_numpy

.github/workflows/windows.yml

@@ -76,7 +76,7 @@ jobs:
     #   * add 'live-debug-win' to your PR title
     #   * push something to your PR branch (note that just re-running the pipeline disregards the title update)
     - name: live debug session on failure (manual steps required, check `.github/windows.yml`)
-      if: failure() && contains(github.event.pull_request.title, 'live-debug-win')
+      if: failure()
       uses: mxschmitt/action-tmate@v3
 
     - name: Upload build artifact

.gitmodules

@@ -25,3 +25,6 @@
 [submodule "external/eigen"]
 	path = external/eigen
 	url = https://gitlab.com/libeigen/eigen.git
+[submodule "external/nanobind"]
+	path = external/nanobind
+	url = https://github.com/wjakob/nanobind

CMakeLists.txt

@@ -432,6 +432,9 @@ endif()
 if(NRN_ENABLE_PYTHON)
   # Make sure the USE_PYTHON macro is defined in the C++ code
   list(APPEND NRN_COMPILE_DEFS USE_PYTHON)
+  # Ensure nanobind is there, but dont import, we don't want its CMake
+  nrn_add_external_project(nanobind False)
+  include(NanoBindMinimal)
 endif()
 
 # =============================================================================
@@ -572,6 +575,10 @@ endif()
 # =============================================================================
 add_subdirectory(src/sparse13)
 add_subdirectory(src/gnu)
+if(NRN_ENABLE_PYTHON)
+  add_subdirectory(src/nrnpython)
+endif()
+
 add_subdirectory(src/nrniv)
 
 # Collect the environment variables that are needed to execute NEURON from the build directory. This
@@ -599,10 +606,6 @@ if(NRN_ENABLE_PYTHON)
 endif()
 add_subdirectory(bin)
 
-if(NRN_ENABLE_PYTHON)
-  add_subdirectory(src/nrnpython)
-endif()
-
 if(NRN_MACOS_BUILD)
   add_subdirectory(src/mac)
 endif()

cmake/ExternalProjectHelper.cmake

@@ -29,7 +29,7 @@ function(nrn_initialize_submodule path)
   endif()
   message(STATUS "Sub-module : missing ${path} : running git submodule update --init")
   execute_process(
-    COMMAND ${GIT_EXECUTABLE} submodule update --init -- ${path}
+    COMMAND ${GIT_EXECUTABLE} submodule update --recursive --init -- ${path}
     WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
     RESULT_VARIABLE ret)
   if(NOT ret EQUAL 0)

cmake/NanoBindMinimal.cmake

@@ -0,0 +1,21 @@
+# =============================================================================
+# minimal nanobind interface
+# =============================================================================
+
+set(NB_DIR ${PROJECT_SOURCE_DIR}/external/nanobind)
+
+function(make_nanobind_target TARGET_NAME PYINC)
+  add_library(${TARGET_NAME} STATIC ${NB_DIR}/src/nb_combined.cpp)
+  target_include_directories(${TARGET_NAME} SYSTEM PUBLIC ${NB_DIR}/include)
+  target_include_directories(${TARGET_NAME} SYSTEM PRIVATE ${NB_DIR}/ext/robin_map/include ${PYINC})
+  if(MSVC)
+    # Do not complain about vsnprintf
+    target_compile_definitions(${TARGET_NAME} PRIVATE -D_CRT_SECURE_NO_WARNINGS)
+  else()
+    # Generally needed to handle type punning in Python code
+    target_compile_options(${TARGET_NAME} PRIVATE -fno-strict-aliasing)
+  endif()
+  target_compile_features(${TARGET_NAME} PUBLIC cxx_std_17)
+  set_property(TARGET ${TARGET_NAME} PROPERTY POSITION_INDEPENDENT_CODE True)
+  set_property(TARGET ${TARGET_NAME} PROPERTY INTERPROCEDURAL_OPTIMIZATION_RELEASE ON)
+endfunction()

src/nrnpython/CMakeLists.txt

@@ -49,11 +49,18 @@ if(NRN_ENABLE_PYTHON_DYNAMIC)
     list(GET NRN_PYTHON_VERSIONS ${val} pyver)
     list(GET NRN_PYTHON_INCLUDES ${val} pyinc)
     list(GET NRN_PYTHON_LIBRARIES ${val} pylib)
+    set(nanobind_target "nanobind_py${pyver}")
+    make_nanobind_target(${nanobind_target} ${pyinc})
     add_library(nrnpython${pyver} SHARED ${NRNPYTHON_FILES_LIST})
+    if(MSVC)
+      set_property(TARGET nrnpython${pyver} PROPERTY WINDOWS_EXPORT_ALL_SYMBOLS ON)
+      set(CMAKE_SUPPORT_WINDOWS_EXPORT_ALL_SYMBOLS 1)
+    endif()
     target_include_directories(nrnpython${pyver} BEFORE PUBLIC ${pyinc} ${INCLUDE_DIRS})
-    target_link_libraries(nrnpython${pyver} nrniv_lib ${Readline_LIBRARY})
+    target_link_libraries(nrnpython${pyver} PUBLIC nrniv_lib)
+    target_link_libraries(nrnpython${pyver} PRIVATE ${Readline_LIBRARY} ${nanobind_target})
     if(NRN_LINK_AGAINST_PYTHON)
-      target_link_libraries(nrnpython${pyver} ${pylib})
+      target_link_libraries(nrnpython${pyver} PUBLIC ${pylib})
     endif()
     add_dependencies(nrnpython${pyver} nrniv_lib)
     list(APPEND nrnpython_lib_list nrnpython${pyver})
@@ -68,6 +75,8 @@ else()
   target_link_libraries(nrnpython ${NRN_DEFAULT_PYTHON_LIBRARIES})
   target_include_directories(nrnpython PUBLIC ${PROJECT_SOURCE_DIR}/${NRN_3RDPARTY_DIR}/eigen)
   target_include_directories(nrnpython PUBLIC ${PROJECT_BINARY_DIR}/src/nrniv/oc_generated)
+  make_nanobind_target(nanobind ${NRN_DEFAULT_PYTHON_INCLUDES})
+  target_link_libraries(nrnpython nanobind)
 endif()
 
 configure_file(_config_params.py.in "${PROJECT_BINARY_DIR}/lib/python/neuron/_config_params.py"

src/nrnpython/nrnpy_hoc.cpp

@@ -21,6 +21,10 @@
 #include <vector>
 #include <sstream>
 
+#include <nanobind/nanobind.h>
+
+namespace nb = nanobind;
+
 extern PyTypeObject* psection_type;
 extern std::vector<const char*> py_exposed_classes;
 
@@ -68,8 +72,6 @@
 extern Object** (*nrnpy_vec_to_python_p_)(void*);
 extern Object** (*nrnpy_vec_as_numpy_helper_)(int, double*);
 extern Object* (*nrnpy_rvp_rxd_to_callable)(Object*);
-extern "C" int nrnpy_set_vec_as_numpy(PyObject* (*p)(int, double*) );  // called by ctypes.
-extern "C" int nrnpy_set_gui_callback(PyObject*);
 extern Symbol* ivoc_alias_lookup(const char* name, Object* ob);
 class NetCon;
 extern int nrn_netcon_weight(NetCon*, double**);
@@ -2460,60 +2462,65 @@
     return static_cast<char*>(data);
 }
 
+
+inline double pyobj_to_double_or_fail(PyObject* obj, long obj_id) {
+    if (!PyNumber_Check(obj)) {
+        char buf[50];
+        Sprintf(buf, "item %d is not a valid number", obj_id);
+        hoc_execerror(buf, 0);
+    }
+    return PyFloat_AsDouble(obj);
+}
+
 static IvocVect* nrnpy_vec_from_python(void* v) {
     Vect* hv = (Vect*) v;
     //	printf("%s.from_array\n", hoc_object_name(hv->obj_));
     Object* ho = *hoc_objgetarg(1);
     if (ho->ctemplate->sym != nrnpy_pyobj_sym_) {
         hoc_execerror(hoc_object_name(ho), " is not a PythonObject");
     }
-    PyObject* po = nrnpy_hoc2pyobject(ho);
-    Py_INCREF(po);
-    if (!PySequence_Check(po)) {
-        if (!PyIter_Check(po)) {
+    // We borrow the list, so there's an INCREF and all items are alive
+    nb::object po = nb::borrow(nrnpy_hoc2pyobject(ho));
+
+    // If it's not a sequence, try iterating over it
+    if (!PySequence_Check(po.ptr())) {
+        if (!PyIter_Check(po.ptr())) {
             hoc_execerror(hoc_object_name(ho),
                           " does not support the Python Sequence or Iterator protocol");
         }
-        PyObject* iterator = PyObject_GetIter(po);
-        assert(iterator != NULL);
-        int i = 0;
-        PyObject* p;
-        while ((p = PyIter_Next(iterator)) != NULL) {
-            if (!PyNumber_Check(p)) {
-                char buf[50];
-                Sprintf(buf, "item %d not a number", i);
-                hoc_execerror(buf, 0);
-            }
-            hv->push_back(PyFloat_AsDouble(p));
-            Py_DECREF(p);
-            ++i;
+        long i = 0;
+        for (nb::handle item: po) {
+            hv->push_back(pyobj_to_double_or_fail(item.ptr(), i));
+            i++;
+        }
+        return hv;
+    }
+
+    int size = nb::len(po);
+    hv->resize(size);
+    double* x = vector_vec(hv);
+
+    // If sequence provides __array_interface__ use it
+    long stride;
+    char* array_interface_ptr = double_array_interface(po.ptr(), stride);
+    if (array_interface_ptr) {
+        for (int i = 0, j = 0; i < size; ++i, j += stride) {
+            x[i] = *(double*) (array_interface_ptr + j);
+        }
+        return hv;
+    }
+
+    // If it's a normal list, convert to the good type so operator[] is more efficient
+    if (PyList_Check(po.ptr())) {
+        nb::list list_obj{std::move(po)};
+        for (long i = 0; i < size; ++i) {
+            x[i] = pyobj_to_double_or_fail(list_obj[i].ptr(), i);
         }
-        Py_DECREF(iterator);
     } else {
-        int size = PySequence_Size(po);
-        //		printf("size = %d\n", size);
-        hv->resize(size);
-        double* x = vector_vec(hv);
-        long stride;
-        char* y = double_array_interface(po, stride);
-        if (y) {
-            for (int i = 0, j = 0; i < size; ++i, j += stride) {
-                x[i] = *(double*) (y + j);
-            }
-        } else {
-            for (int i = 0; i < size; ++i) {
-                PyObject* p = PySequence_GetItem(po, i);
-                if (!PyNumber_Check(p)) {
-                    char buf[50];
-                    Sprintf(buf, "item %d not a number", i);
-                    hoc_execerror(buf, 0);
-                }
-                x[i] = PyFloat_AsDouble(p);
-                Py_DECREF(p);
-            }
+        for (long i = 0; i < size; ++i) {
+            x[i] = pyobj_to_double_or_fail(po[i].ptr(), i);
         }
     }
-    Py_DECREF(po);
     return hv;
 }