From 3d321562b4cc1b85e821c238632f73802c7c9e01 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Wed, 20 Nov 2024 14:40:26 +0100 Subject: [PATCH 1/6] Use nb::make_tuple --- src/nrnpython/nrnpy_p2h.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/nrnpython/nrnpy_p2h.cpp b/src/nrnpython/nrnpy_p2h.cpp index cde00ef0a2..564446dbf5 100644 --- a/src/nrnpython/nrnpy_p2h.cpp +++ b/src/nrnpython/nrnpy_p2h.cpp @@ -428,9 +428,7 @@ static Object* callable_with_args(Object* ho, int narg) { } } - auto r = nb::steal(PyTuple_New(2)); - PyTuple_SetItem(r.ptr(), 1, args.release().ptr()); - PyTuple_SetItem(r.ptr(), 0, po.release().ptr()); + nb::tuple r = nb::make_tuple(po, args); Object* hr = nrnpy_po2ho(r.release().ptr()); @@ -885,9 +883,7 @@ static Object* py_alltoall_type(int size, int type) { sdispl = mk_displ(scnt.data()); rdispl = mk_displ(rcnt); if (size < 0) { - pdest = PyTuple_New(2); - PyTuple_SetItem(pdest, 0, Py_BuildValue("l", (long) sdispl[np])); - PyTuple_SetItem(pdest, 1, Py_BuildValue("l", (long) rdispl[np])); + pdest = nb::make_tuple(sdispl[np], rdispl[np]).release().ptr() delete[] sdispl; delete[] rcnt; delete[] rdispl; From b1d14dc500358bc1dbb0a004be7ce47ada879191 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Wed, 20 Nov 2024 15:17:57 +0100 Subject: [PATCH 2/6] Simplify hocpickle_reduce --- src/nrnpython/nrnpy_hoc.cpp | 48 ++++++++----------------------------- 1 file changed, 10 insertions(+), 38 deletions(-) diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index 07ec35c158..c2f9bbd690 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -2909,58 +2909,30 @@ static PyObject* hocpickle_reduce(PyObject* self, PyObject* args) { PyHocObject* pho = (PyHocObject*) self; if (!is_obj_type(pho->ho_, "Vector")) { PyErr_SetString(PyExc_TypeError, "HocObject: Only Vector instance can be pickled"); - return NULL; + return nullptr; } Vect* vec = (Vect*) pho->ho_->u.this_pointer; // neuron module has a _pkl method that returns h.Vector(0) - PyObject* mod = PyImport_ImportModule("neuron"); - if (mod == NULL) { - return NULL; - } - PyObject* obj = PyObject_GetAttrString(mod, "_pkl"); - Py_DECREF(mod); - if (obj == NULL) { + nb::module_ mod = nb::module_::import_("neuron"); + nb::object obj = mod.attr("_pkl"); + if (!obj) { PyErr_SetString(PyExc_Exception, "neuron module has no _pkl method."); - return NULL; + return nullptr; } - PyObject* ret = PyTuple_New(3); - if (ret == NULL) { - return NULL; - } - PyTuple_SET_ITEM(ret, 0, obj); - PyTuple_SET_ITEM(ret, 1, Py_BuildValue("(N)", PyInt_FromLong(0))); // see numpy implementation if more ret[1] stuff needed in case we // pickle anything but a hoc Vector. I don't think ret[1] can be None. // Fill object's state. Tuple with 4 args: // pickle version, 2.0 bytes to determine if swapbytes needed, // vector size, string data - PyObject* state = PyTuple_New(4); - if (state == NULL) { - Py_DECREF(ret); - return NULL; - } - PyTuple_SET_ITEM(state, 0, PyInt_FromLong(1)); double x = 2.0; - PyObject* str = PyBytes_FromStringAndSize((const char*) (&x), sizeof(double)); - if (str == NULL) { - Py_DECREF(ret); - Py_DECREF(state); - return NULL; - } - PyTuple_SET_ITEM(state, 1, str); - PyTuple_SET_ITEM(state, 2, PyInt_FromLong(vec->size())); - str = PyBytes_FromStringAndSize((const char*) vector_vec(vec), vec->size() * sizeof(double)); - if (str == NULL) { - Py_DECREF(ret); - Py_DECREF(state); - return NULL; - } - PyTuple_SET_ITEM(state, 3, str); - PyTuple_SET_ITEM(ret, 2, state); - return ret; + nb::bytes str((const void*)(&x), sizeof(double)); + nb::bytes str1(vec->data(), vec->size() * sizeof(double)); + nb::tuple state = nb::make_tuple(1, str, vec->size(), str1); + + return nb::make_tuple(obj, nb::make_tuple(0), state).release().ptr(); } static PyObject* hocpickle_reduce_safe(PyObject* self, PyObject* args) { From 9280833e188b80508c3898f124657ba8efbec12a Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Wed, 20 Nov 2024 17:49:53 +0100 Subject: [PATCH 3/6] Format --- src/nrnpython/nrnpy_hoc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index e2228dec92..248b163112 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -2920,7 +2920,7 @@ static PyObject* hocpickle_reduce(PyObject* self, PyObject* args) { // pickle version, 2.0 bytes to determine if swapbytes needed, // vector size, string data double x = 2.0; - nb::bytes str((const void*)(&x), sizeof(double)); + nb::bytes str((const void*) (&x), sizeof(double)); nb::bytes str1(vec->data(), vec->size() * sizeof(double)); nb::tuple state = nb::make_tuple(1, str, vec->size(), str1); From 69f2d47ba90ad754e440b2a1c1b2aa5088ccebd7 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Thu, 21 Nov 2024 16:25:42 +0100 Subject: [PATCH 4/6] Fix review --- src/nrnpython/nrnpy_hoc.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index 248b163112..f9e189ef79 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -2907,6 +2907,9 @@ static PyObject* hocpickle_reduce(PyObject* self, PyObject* args) { // neuron module has a _pkl method that returns h.Vector(0) nb::module_ mod = nb::module_::import_("neuron"); + if (!mod) { + return nullptr; + } nb::object obj = mod.attr("_pkl"); if (!obj) { PyErr_SetString(PyExc_Exception, "neuron module has no _pkl method."); @@ -2917,12 +2920,12 @@ static PyObject* hocpickle_reduce(PyObject* self, PyObject* args) { // pickle anything but a hoc Vector. I don't think ret[1] can be None. // Fill object's state. Tuple with 4 args: - // pickle version, 2.0 bytes to determine if swapbytes needed, + // pickle version, endianness sentinel, // vector size, string data double x = 2.0; - nb::bytes str((const void*) (&x), sizeof(double)); - nb::bytes str1(vec->data(), vec->size() * sizeof(double)); - nb::tuple state = nb::make_tuple(1, str, vec->size(), str1); + nb::bytes byte_order((const void*) (&x), sizeof(double)); + nb::bytes vec_data(vec->data(), vec->size() * sizeof(double)); + nb::tuple state = nb::make_tuple(1, byte_order, vec->size(), vec_data); return nb::make_tuple(obj, nb::make_tuple(0), state).release().ptr(); } From 85584f4e9419bb3a52a12ade47adf055110ccd70 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Thu, 21 Nov 2024 16:28:17 +0100 Subject: [PATCH 5/6] Update src/nrnpython/nrnpy_hoc.cpp Co-authored-by: Luc Grosheintz --- src/nrnpython/nrnpy_hoc.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index 988f929eb7..f0f05a84d6 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -2922,6 +2922,8 @@ static PyObject* hocpickle_reduce(PyObject* self, PyObject* args) { // Fill object's state. Tuple with 4 args: // pickle version, endianness sentinel, // vector size, string data + // + // To be able to read data on a system with different endianness, a sentinel is added, the convention is that the value of the sentinel is `2.0` (when cast to a double). Therefore, if the machine reads the sentinel and it's not `2.0` it know that it needs to swap the bytes of all doubles in the payload. double x = 2.0; nb::bytes byte_order((const void*) (&x), sizeof(double)); nb::bytes vec_data(vec->data(), vec->size() * sizeof(double)); From 48de5be34b3c6c64b1b9808d971629e7c57f2798 Mon Sep 17 00:00:00 2001 From: Nicolas Cornu Date: Thu, 21 Nov 2024 16:31:13 +0100 Subject: [PATCH 6/6] format --- src/nrnpython/nrnpy_hoc.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/nrnpython/nrnpy_hoc.cpp b/src/nrnpython/nrnpy_hoc.cpp index f0f05a84d6..e0932452f1 100644 --- a/src/nrnpython/nrnpy_hoc.cpp +++ b/src/nrnpython/nrnpy_hoc.cpp @@ -2923,7 +2923,10 @@ static PyObject* hocpickle_reduce(PyObject* self, PyObject* args) { // pickle version, endianness sentinel, // vector size, string data // - // To be able to read data on a system with different endianness, a sentinel is added, the convention is that the value of the sentinel is `2.0` (when cast to a double). Therefore, if the machine reads the sentinel and it's not `2.0` it know that it needs to swap the bytes of all doubles in the payload. + // To be able to read data on a system with different endianness, a sentinel is added, the + // convention is that the value of the sentinel is `2.0` (when cast to a double). Therefore, if + // the machine reads the sentinel and it's not `2.0` it know that it needs to swap the bytes of + // all doubles in the payload. double x = 2.0; nb::bytes byte_order((const void*) (&x), sizeof(double)); nb::bytes vec_data(vec->data(), vec->size() * sizeof(double));