From c5fe60ab6aea039eaf6e371ccfcccb0c5b100b11 Mon Sep 17 00:00:00 2001 From: Anthony Geay Date: Thu, 6 Dec 2018 16:06:36 +0100 Subject: [PATCH] Bug correction in the medcoupling <-> numpy layer --- src/MEDCoupling/MEDCouplingMemArray.hxx | 1 + src/MEDCoupling/MEDCouplingMemArray.txx | 10 ++++++++++ src/MEDCoupling/MEDCouplingRefCountObject.hxx | 3 ++- .../MEDCouplingDataArrayTraits.hxx | 4 +++- .../MEDCouplingDataArrayTypemaps.i | 14 ++++++++++---- src/MEDCoupling_Swig/MEDCouplingNumPyTest.py | 18 +++++++++++++++++- 6 files changed, 43 insertions(+), 7 deletions(-) diff --git a/src/MEDCoupling/MEDCouplingMemArray.hxx b/src/MEDCoupling/MEDCouplingMemArray.hxx index de2101fea..3887de629 100644 --- a/src/MEDCoupling/MEDCouplingMemArray.hxx +++ b/src/MEDCoupling/MEDCouplingMemArray.hxx @@ -112,6 +112,7 @@ namespace MEDCoupling public: static void CPPDeallocator(void *pt, void *param); static void CDeallocator(void *pt, void *param); + static void COffsetDeallocator(void *pt, void *param); private: static void DestroyPointer(T *pt, Deallocator dealloc, void *param); static Deallocator BuildFromType(DeallocType type); diff --git a/src/MEDCoupling/MEDCouplingMemArray.txx b/src/MEDCoupling/MEDCouplingMemArray.txx index e43325aa3..f2c2f4690 100644 --- a/src/MEDCoupling/MEDCouplingMemArray.txx +++ b/src/MEDCoupling/MEDCouplingMemArray.txx @@ -437,6 +437,14 @@ namespace MEDCoupling free(pt); } + template + void MemArray::COffsetDeallocator(void *pt, void *param) + { + int64_t *offset(reinterpret_cast(param)); + char *ptcast(reinterpret_cast(pt)); + free(ptcast+*offset); + } + template typename MemArray::Deallocator MemArray::BuildFromType(DeallocType type) { @@ -446,6 +454,8 @@ namespace MEDCoupling return CPPDeallocator; case DeallocType::C_DEALLOC: return CDeallocator; + case DeallocType::C_DEALLOC_WITH_OFFSET: + return COffsetDeallocator; default: throw INTERP_KERNEL::Exception("Invalid deallocation requested ! Unrecognized enum DeallocType !"); } diff --git a/src/MEDCoupling/MEDCouplingRefCountObject.hxx b/src/MEDCoupling/MEDCouplingRefCountObject.hxx index dcece7d5b..5473a9a04 100644 --- a/src/MEDCoupling/MEDCouplingRefCountObject.hxx +++ b/src/MEDCoupling/MEDCouplingRefCountObject.hxx @@ -34,7 +34,8 @@ namespace MEDCoupling enum class DeallocType { C_DEALLOC = 2, - CPP_DEALLOC = 3 + CPP_DEALLOC = 3, + C_DEALLOC_WITH_OFFSET = 4 }; //! The various spatial discretization of a field diff --git a/src/MEDCoupling_Swig/MEDCouplingDataArrayTraits.hxx b/src/MEDCoupling_Swig/MEDCouplingDataArrayTraits.hxx index 359bc7ecd..50b0f9b41 100644 --- a/src/MEDCoupling_Swig/MEDCouplingDataArrayTraits.hxx +++ b/src/MEDCoupling_Swig/MEDCouplingDataArrayTraits.hxx @@ -55,7 +55,9 @@ void numarrdeal(void *pt, void *wron) { typedef void (*MyDeallocator)(void *,void *); MyDeallocator deall=(MyDeallocator)wronc[1]; - deall(pt,NULL); + int64_t *offset=reinterpret_cast(wronc[2]); + deall(pt,offset); + delete offset; Py_XDECREF(weakRefOnOwner); } delete [] wronc; diff --git a/src/MEDCoupling_Swig/MEDCouplingDataArrayTypemaps.i b/src/MEDCoupling_Swig/MEDCouplingDataArrayTypemaps.i index 3a6bbcdca..c56510f7a 100644 --- a/src/MEDCoupling_Swig/MEDCouplingDataArrayTypemaps.i +++ b/src/MEDCoupling_Swig/MEDCouplingDataArrayTypemaps.i @@ -199,11 +199,17 @@ MCData *BuildNewInstance(PyObject *elt0, int npyObjectType, PyTypeObject *pytype } else { - ret->useArray(reinterpret_cast(data),true,MEDCoupling::DeallocType::C_DEALLOC,sz0,sz1); + ret->useArray(reinterpret_cast(data),true,MEDCoupling::DeallocType::C_DEALLOC_WITH_OFFSET,sz0,sz1); PyObject *ref=PyWeakref_NewRef(reinterpret_cast(eltOwning),NULL); - typename MEDCoupling::MemArray::Deallocator tmp(MEDCoupling::MemArray::CDeallocator); + typename MEDCoupling::MemArray::Deallocator tmp(MEDCoupling::MemArray::COffsetDeallocator); void **tmp2 = reinterpret_cast(&tmp); // MSVC2010 does not support constructor() - void **objs=new void *[2]; objs[0]=ref; objs[1]=*tmp2; + const char *dataEltOwning(PyArray_BYTES(eltOwning));//In case of input array is a sub array of a 2D,3D... array there is an offset + int64_t offset(0); + if(data!=dataEltOwning) + { + offset=data>dataEltOwning?-((int64_t)(std::distance(dataEltOwning,data))):(int64_t)std::distance(data,dataEltOwning); + } + void **objs=new void *[3]; objs[0]=ref; objs[1]=*tmp2; objs[2]=new int64_t(offset); mma.setParameterForDeallocator(objs); mma.setSpecificDeallocator(numarrdeal); } @@ -308,7 +314,7 @@ PyObject *ToNumPyArrayUnderground(MCData *self, int npyObjectType, const char *M PyObject *ref(PyWeakref_NewRef(ret,NULL)); typename MEDCoupling::MemArray::Deallocator tmp(mem.getDeallocator()); void **tmp2 = reinterpret_cast(&tmp); // MSVC2010 does not support constructor() - void **objs=new void *[2]; objs[0]=reinterpret_cast(ref); objs[1]=*tmp2; + void **objs=new void *[3]; objs[0]=reinterpret_cast(ref); objs[1]=*tmp2; objs[2]=new int64_t(0); mem.setParameterForDeallocator(objs); mem.setSpecificDeallocator(numarrdeal); return ret; diff --git a/src/MEDCoupling_Swig/MEDCouplingNumPyTest.py b/src/MEDCoupling_Swig/MEDCouplingNumPyTest.py index 0cfc225e3..6ce7868ef 100644 --- a/src/MEDCoupling_Swig/MEDCouplingNumPyTest.py +++ b/src/MEDCoupling_Swig/MEDCouplingNumPyTest.py @@ -23,7 +23,7 @@ import sys if sys.platform == "win32": from MEDCouplingCompat import * else: - from MEDCoupling import * + from medcoupling import * if MEDCouplingHasNumPyBindings(): from numpy import * @@ -1062,6 +1062,22 @@ class MEDCouplingNumPyTest(unittest.TestCase): self.assertTrue(not b.flags["OWNDATA"]) pass + @unittest.skipUnless(MEDCouplingHasNumPyBindings(),"requires numpy") + def test41(self): + """ This non regression test is focused on a numpy subarray of a bigger numpy array. Typically a 1D array coming from a 2D array. When medcoupling takes the ownership, medcoupling must store an offset to deallocate correctly the pointer. The pointer of medcoupling array is NOT the pointer to be transmited to free. The offset is typically the distance between the start of the main 2D array and the start of 1D array medcouplingized.""" + import numpy as np + array = np.array([[1,2,3,10],[4,5,6,20],[7,8,9,30]],dtype=np.float64) # create a 2D array + b = array[2] # b data pointer starts at array+2*4*sizeof(float64) so offset is expected to be equal to -2*4*sizeof(float64)=-64 + self.assertTrue(array.flags["OWNDATA"]) + self.assertTrue(not b.flags["OWNDATA"]) + d=DataArrayDouble(b) + self.assertTrue(not array.flags["OWNDATA"]) + self.assertTrue(not b.flags["OWNDATA"]) + del b ; gc.collect() + del array ; gc.collect() + del d ; gc.collect() # important : destroy d after b and array to be sure to let the ownership to d. + pass + def setUp(self): pass pass -- 2.39.2