October 2014 - petsc-dev - mailman3.cels.anl.gov

Re: [petsc-dev] [petsc-maint] Memory leak in some usage of MatAXPY
by Barry Smith 17 Oct '14

17 Oct '14

> On Oct 17, 2014, at 2:33 PM, Hong <hzhang(a)mcs.anl.gov> wrote: > > Barry, > Current implementation of MatAXPY_MPIAIJ() (and other matrix types) > for SUBSET_NONZERO_PATTERN creates maps > xtoy and XtoY and attaches these maps to matrix Y, which cannot be > reused and is unnecessarily complicated (I wrote it many years ago). > > A simple way for this case is calling MatAXPY_Basic() > which adds aX to Y via MatSetValues() without changing data structure of Y. I’m afraid that it will be horrifically slow since it will be constantly reallocating for new entries. What about keeping your custom code but simply freeing the maps at the end of the MatAXPY instead of attaching them? Barry > > If you agree, I like make this change: > > $ git diff src/mat/utils/axpy.c > diff --git a/src/mat/utils/axpy.c b/src/mat/utils/axpy.c > index 23caac7..c629c9c 100644 > --- a/src/mat/utils/axpy.c > +++ b/src/mat/utils/axpy.c > @@ -35,7 +35,7 @@ PetscErrorCode MatAXPY(Mat Y,PetscScalar a,Mat > X,MatStructure str) > > - if (Y->ops->axpy) { > + if (Y->ops->axpy && str != SUBSET_NONZERO_PATTERN) { > ierr = (*Y->ops->axpy)(Y,a,X,str);CHKERRQ(ierr); > } else { > ierr = MatAXPY_Basic(Y,a,X,str);CHKERRQ(ierr); > > > and remove xtoy and XtoY from struct Mat_xxx. > > This fixes the memory leak reported by Jose. > > Hong > > ---------- Forwarded message ---------- > From: Hong <hzhang(a)mcs.anl.gov> > Date: Thu, Oct 16, 2014 at 12:52 PM > Subject: Re: [petsc-maint] Memory leak in some usage of MatAXPY > To: "Jose E. Roman" <jroman(a)dsic.upv.es> > Cc: petsc-maint Help <petsc-maint(a)mcs.anl.gov>, Carmen Campos > <mccampos(a)dsic.upv.es> > > > Jose: > I can repeat the leak with your code. I'll investigate and fix it on > maint. I will let you know once I'm done. > > Hong > >> We are experiencing a memory leak in MatAXPY, appearing only in some specific usages: >> - with more than one process >> - with SUBSET_NONZERO_PATTERN >> - with several calls to MatAXPY >> >> The problem appears in master and maint, and also in petsc-3.4. >> >> The situation is the following: imagine you want to accumulate two matrices onto a third one. The third one is chosen to have the sparse pattern containing the pattern of the other two (so that SUBSET_NONZERO_PATTERN can be used). Then calling MatAXPY twice with subset+subset gives a memory leak, but different+subset does not. >> >> I am attaching a sample code that reproduces the problem. It has the command-line options: >> -different: use DIFFERENT_NONZERO_PATTERN for the first MatAXPY (instead of subset) >> -skip: omit the second MatAXPY >> >> $ mpirun -np 2 ./testMatAXPY --> memory leak >> $ mpirun -np 2 ./testMatAXPY -different --> ok >> $ mpirun -np 2 ./testMatAXPY -skip --> ok >> $ mpirun -np 2 ./testMatAXPY -different -skip --> ok >> >> Jose >>

1 0

Re: [petsc-dev] Backend independent VecGetArray for GPUs
by Karl Rupp 17 Oct '14

17 Oct '14

On 10/17/2014 09:11 PM, Barry Smith wrote: > >> On Oct 17, 2014, at 2:05 PM, Karl Rupp <rupp(a)iue.tuwien.ac.at> wrote: >> >> >>> >>> But you won’t want to have to create the CUSP or ViennaCL objects on the fly each time from the CUDA/OpenCL “raw pointers”? >> >> why not? I can just 'wrap' an existing memory buffer for use with the respective operations provided with the respective library. > > So you mean like VecPlaceArray()? Ok. Yes. > This is a big refactorization, probably more than what Lisandro is looking for in the short run. Definitely. > So what do we do in the short run, provide VecCUSPGetCUDAArray() and VecViennaCLGetOpenCLArray()? I’m ok with that. Agree'd. These are just simple wrappers on top of VecCUSPGetArrayRead()/VecCUSPGetArrayReadWrite() and VecViennaCLGetArrayRead()/VecViennaCLGetArrayReadWrite() I can add these over the weekend. Best regards, Karli

1 0

Re: [petsc-dev] Backend independent VecGetArray for GPUs
by Barry Smith 17 Oct '14

17 Oct '14

> On Oct 17, 2014, at 2:05 PM, Karl Rupp <rupp(a)iue.tuwien.ac.at> wrote: > > >> >> But you won’t want to have to create the CUSP or ViennaCL objects on the fly each time from the CUDA/OpenCL “raw pointers”? > > why not? I can just 'wrap' an existing memory buffer for use with the respective operations provided with the respective library. So you mean like VecPlaceArray()? Ok. This is a big refactorization, probably more than what Lisandro is looking for in the short run. So what do we do in the short run, provide VecCUSPGetCUDAArray() and VecViennaCLGetOpenCLArray()? I’m ok with that. Barry > Note that the cost of setting up such wrappers is negligible compared to the cost of launching a CUDA or OpenCL kernel. > > Best regards, > Karli >

1 0

Re: [petsc-dev] Backend independent VecGetArray for GPUs
by Karl Rupp 17 Oct '14

17 Oct '14

> > But you won’t want to have to create the CUSP or ViennaCL objects on the fly each time from the CUDA/OpenCL “raw pointers”? why not? I can just 'wrap' an existing memory buffer for use with the respective operations provided with the respective library. Note that the cost of setting up such wrappers is negligible compared to the cost of launching a CUDA or OpenCL kernel. Best regards, Karli

1 0

Re: [petsc-dev] Backend independent VecGetArray for GPUs
by Barry Smith 17 Oct '14

17 Oct '14

> On Oct 17, 2014, at 12:48 PM, Karl Rupp <rupp(a)iue.tuwien.ac.at> wrote: > > >> So in the compiled C/C++ world >> >>>> VecCUDAGetArray() >>>> VecOpenCLGetArray() >> >> would essentially call VecCUSPGetArray() or VecViennaCLGetArray() and then pull the “raw” pointer out of that beasty? But we really don’t need VecCUDAGetArray() and VecOpenCLGetArray() in the C/C++ world since the user can simply call VecCUSPGetArray() or VecViennaCLGetArray() and then use the CUSP or ViennaCL specific way of pulling out the “raw” CUSP or OpenCL pointer, is this correct? > > With the current implementation, this is correct. > However, I would like to decouple the use of CUDA and OpenCL buffers from the respective packages. The current implementation in PETSc is such that the buffers are burried in package-specific types (cusp::device_vector, viennacl::vector), from which they can be retrieved. For better modularity, I would like to invert this relationship and store the native memory handles. CUSP and ViennaCL then only kick in when certain operations are requested, But you won’t want to have to create the CUSP or ViennaCL objects on the fly each time from the CUDA/OpenCL “raw pointers”? This means you would still need to store the CUSP/ViennaCL object along with the “raw pointer”, so how is that different from now? > for example a simple dot product, or a full-fledged preconditioner. This allows for a much better mix&match: One could for example use the CUDA backend in ViennaCL for certain operations not provided by CUSP (e.g. VecMDot()), and combine this with the AMG preconditioner in CUSP. One can later add any other library that is able to work with such raw memory buffers (a property absolutely essential for any library interaction, but too often totally forgotten in the C++ world...) > > >> Now moving onto Python, presumably CUSP and ViennaCL do not have python bindings? (Why the heck not?) And thus there is no “natural” thing for VecCUSPGetArray() or VecViennaCLGetArray() to return in Python? > > Depends... The natural thing for the ViennaCL case is pyViennaCL (http://viennacl.sourceforge.net/pyviennacl/doc/) but I don't know enough details of Python in order to judge whether this is appropriate in this setting. For CUSP there's no "natural" thing (as far as I know) other than a raw PyCUDA handle. That's okay for vectors, but it gets pretty delicate with sparse matrices using GPU-specific memory layouts. > > >> So, the conclusion is that since CUSP and ViennaCL do not have Python bindings (why the heck not?) this means PETSc needs to extend its API even in C/C++ land? Or would VecCUDAGetArray() and VecOpenCLGetArray() only exist in Python? An alternative is to introduce into Python a CUSP array class and a Vienna CL array class that only does one thing: provides a way to pull out the raw CUDA or OpenCL pointer, then in Python the user would do the same thing as C/C++, first pull out the CUSP/ViennaCL pointer with VecCUSPGetArray() or VecViennaCLGetArray() and then pull out the raw pointer using the little “wrapper class"? > > I think the problem here is that the implementation is too package-centric rather than framework-centric. (CUDA/OpenCL) > > >> One the naming: It should be VecCUSPGetCUDAArray() and VecViennaCLGetOpenCLArray() since the second part of the name is associated with the sub class of the first part of the name (e.g. VecCUSP is a subclass of Vec) Since there is no VecOpenCL subclass, for example, it cannot be VecOpenCLGetArray() >> >> What am I missing? > > The current implementation would indeed require > Vec[PACKAGE]Get[FRAMEWORK]Array() > I guess you can see the ugliness and poor scalability... ;-) > > Best regards, > Karli >

1 0

Re: [petsc-dev] Backend independent VecGetArray for GPUs
by Karl Rupp 17 Oct '14

17 Oct '14

> So in the compiled C/C++ world > >>> VecCUDAGetArray() >>> VecOpenCLGetArray() > > would essentially call VecCUSPGetArray() or VecViennaCLGetArray() and then pull the “raw” pointer out of that beasty? But we really don’t need VecCUDAGetArray() and VecOpenCLGetArray() in the C/C++ world since the user can simply call VecCUSPGetArray() or VecViennaCLGetArray() and then use the CUSP or ViennaCL specific way of pulling out the “raw” CUSP or OpenCL pointer, is this correct? With the current implementation, this is correct. However, I would like to decouple the use of CUDA and OpenCL buffers from the respective packages. The current implementation in PETSc is such that the buffers are burried in package-specific types (cusp::device_vector, viennacl::vector), from which they can be retrieved. For better modularity, I would like to invert this relationship and store the native memory handles. CUSP and ViennaCL then only kick in when certain operations are requested, for example a simple dot product, or a full-fledged preconditioner. This allows for a much better mix&match: One could for example use the CUDA backend in ViennaCL for certain operations not provided by CUSP (e.g. VecMDot()), and combine this with the AMG preconditioner in CUSP. One can later add any other library that is able to work with such raw memory buffers (a property absolutely essential for any library interaction, but too often totally forgotten in the C++ world...) > Now moving onto Python, presumably CUSP and ViennaCL do not have python bindings? (Why the heck not?) And thus there is no “natural” thing for VecCUSPGetArray() or VecViennaCLGetArray() to return in Python? Depends... The natural thing for the ViennaCL case is pyViennaCL (http://viennacl.sourceforge.net/pyviennacl/doc/) but I don't know enough details of Python in order to judge whether this is appropriate in this setting. For CUSP there's no "natural" thing (as far as I know) other than a raw PyCUDA handle. That's okay for vectors, but it gets pretty delicate with sparse matrices using GPU-specific memory layouts. > So, the conclusion is that since CUSP and ViennaCL do not have Python bindings (why the heck not?) this means PETSc needs to extend its API even in C/C++ land? Or would VecCUDAGetArray() and VecOpenCLGetArray() only exist in Python? An alternative is to introduce into Python a CUSP array class and a Vienna CL array class that only does one thing: provides a way to pull out the raw CUDA or OpenCL pointer, then in Python the user would do the same thing as C/C++, first pull out the CUSP/ViennaCL pointer with VecCUSPGetArray() or VecViennaCLGetArray() and then pull out the raw pointer using the little “wrapper class"? I think the problem here is that the implementation is too package-centric rather than framework-centric. (CUDA/OpenCL) > One the naming: It should be VecCUSPGetCUDAArray() and VecViennaCLGetOpenCLArray() since the second part of the name is associated with the sub class of the first part of the name (e.g. VecCUSP is a subclass of Vec) Since there is no VecOpenCL subclass, for example, it cannot be VecOpenCLGetArray() > > What am I missing? The current implementation would indeed require Vec[PACKAGE]Get[FRAMEWORK]Array() I guess you can see the ugliness and poor scalability... ;-) Best regards, Karli

1 0

Re: [petsc-dev] Backend independent VecGetArray for GPUs
by Karl Rupp 17 Oct '14

17 Oct '14

On 10/17/2014 06:40 PM, Jed Brown wrote: > Karl Rupp <rupp(a)iue.tuwien.ac.at> writes: >> If this should be provided, then the right names were >> VecCUDAGetArray() >> VecOpenCLGetArray() >> The former returns an object of type PetscScalar* and the latter an >> object of type cl_mem. Because one is stuck with two incompatible >> pointer types here, I'd suggest not to provide a single interface at the >> expense of giving up type safety. > > Agreed. > > Does cl_mem have a "null"? Yes, cl_mem is a typedef for an opaque pointer to a struct. If you pass a cl_mem of 'NULL' to a kernel, the kernel is launched with a 'NULL' pointer for the respective argument. >> Also, even if one gives up type safety, one would immediately have to >> switch-case into a backend-specific implementation again, so there's >> little value in a common interface. > > It's useful to have a non-error path for calling. Can you sketch a use case for this? Best regards, Karli

1 0

Re: [petsc-dev] Backend independent VecGetArray for GPUs
by Barry Smith 17 Oct '14

17 Oct '14

So in the compiled C/C++ world >> VecCUDAGetArray() >> VecOpenCLGetArray() would essentially call VecCUSPGetArray() or VecViennaCLGetArray() and then pull the “raw” pointer out of that beasty? But we really don’t need VecCUDAGetArray() and VecOpenCLGetArray() in the C/C++ world since the user can simply call VecCUSPGetArray() or VecViennaCLGetArray() and then use the CUSP or ViennaCL specific way of pulling out the “raw” CUSP or OpenCL pointer, is this correct? Now moving onto Python, presumably CUSP and ViennaCL do not have python bindings? (Why the heck not?) And thus there is no “natural” thing for VecCUSPGetArray() or VecViennaCLGetArray() to return in Python? So, the conclusion is that since CUSP and ViennaCL do not have Python bindings (why the heck not?) this means PETSc needs to extend its API even in C/C++ land? Or would VecCUDAGetArray() and VecOpenCLGetArray() only exist in Python? An alternative is to introduce into Python a CUSP array class and a Vienna CL array class that only does one thing: provides a way to pull out the raw CUDA or OpenCL pointer, then in Python the user would do the same thing as C/C++, first pull out the CUSP/ViennaCL pointer with VecCUSPGetArray() or VecViennaCLGetArray() and then pull out the raw pointer using the little “wrapper class"? One the naming: It should be VecCUSPGetCUDAArray() and VecViennaCLGetOpenCLArray() since the second part of the name is associated with the sub class of the first part of the name (e.g. VecCUSP is a subclass of Vec) Since there is no VecOpenCL subclass, for example, it cannot be VecOpenCLGetArray() What am I missing? Barry > On Oct 17, 2014, at 11:40 AM, Jed Brown <jed(a)jedbrown.org> wrote: > > Karl Rupp <rupp(a)iue.tuwien.ac.at> writes: >> If this should be provided, then the right names were >> VecCUDAGetArray() >> VecOpenCLGetArray() >> The former returns an object of type PetscScalar* and the latter an >> object of type cl_mem. Because one is stuck with two incompatible >> pointer types here, I'd suggest not to provide a single interface at the >> expense of giving up type safety. > > Agreed. > > Does cl_mem have a "null"? > >> Also, even if one gives up type safety, one would immediately have to >> switch-case into a backend-specific implementation again, so there's >> little value in a common interface. > > It's useful to have a non-error path for calling.

1 0

Re: [petsc-dev] Backend independent VecGetArray for GPUs
by Jed Brown 17 Oct '14

17 Oct '14

Karl Rupp <rupp(a)iue.tuwien.ac.at> writes: > If this should be provided, then the right names were > VecCUDAGetArray() > VecOpenCLGetArray() > The former returns an object of type PetscScalar* and the latter an > object of type cl_mem. Because one is stuck with two incompatible > pointer types here, I'd suggest not to provide a single interface at the > expense of giving up type safety. Agreed. Does cl_mem have a "null"? > Also, even if one gives up type safety, one would immediately have to > switch-case into a backend-specific implementation again, so there's > little value in a common interface. It's useful to have a non-error path for calling.

1 0

Re: [petsc-dev] Backend independent VecGetArray for GPUs
by Karl Rupp 17 Oct '14

17 Oct '14

Hi Ashwin, > After discussion with Lisandro Dalcin, I think that it might be a good > idea for PETSc to provide a routine `VecGetGPUArray` (in place of or in > addition to the current `VecCUSPGetArray`) which returns a raw pointer > to device memory, and lets the user decide what to do with it. > > Do you think this can fit in to PETSc? If so, I already have an > implementation, but can use help with the interface. If this should be provided, then the right names were VecCUDAGetArray() VecOpenCLGetArray() The former returns an object of type PetscScalar* and the latter an object of type cl_mem. Because one is stuck with two incompatible pointer types here, I'd suggest not to provide a single interface at the expense of giving up type safety. Also, even if one gives up type safety, one would immediately have to switch-case into a backend-specific implementation again, so there's little value in a common interface. Best regards, Karli

1 0