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

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

17 Oct '14

On 10/17/2014 09:45 AM, Lisandro Dalcin wrote: > On 17 October 2014 17:54, Dominic Meiser <dmeiser(a)txcorp.com> wrote: >> Hi Ashwin, >> >> Are you suggesting that `VecGetGPUArray*` be added to the `Vec` interface? >> That might be problematic because these methods only makes sense for GPU >> vectors. The interface of `Vec` would then be tied to the PETSc >> configuration. >> > These methods could be always available simply generate an error for > non-GPU vector types. That's a possible approach. Barry, Jed, what's the preferred way of doing this? Make base class interface wider or subtyping via compose and query? > >> An alternative might be to compose the various `VecCUSPGetArray` and related >> methods with `Vec` objects. You could then query these methods and use them >> if available (and handle absence of the methods if needed, e.g. throw an >> exception). This would be easy to add. >> > Yes, however note that what we really need is a backend-independent > way to get the RAW pointer to the GPU buffers. Right now we have calls > to get the CUSP or ViennaCL array, to handle them you need C++ and > depend on CUDA/OpenCL at compile-time. > > In what sense should it be backend-independent? The method name should be independent of the backend? Or the presence of the method should be backend independent? I don't see what your gaining by having the methods in Vec. You still need to handle the error that occurs if you're dealing with a non-GPU vector and you still have to know the vector type in order to interpret the return value (device pointer vs OpenCL buffer). Dominic -- Dominic Meiser Tech-X Corporation 5621 Arapahoe Avenue Boulder, CO 80303 USA Telephone: 303-996-2036 Fax: 303-448-7756 www.txcorp.com

1 0

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

17 Oct '14

On Fri, Oct 17, 2014 at 10:45 AM, Lisandro Dalcin <dalcinl(a)gmail.com> wrote: > On 17 October 2014 17:54, Dominic Meiser <dmeiser(a)txcorp.com> wrote: > > Hi Ashwin, > > > > Are you suggesting that `VecGetGPUArray*` be added to the `Vec` > interface? > > That might be problematic because these methods only makes sense for GPU > > vectors. The interface of `Vec` would then be tied to the PETSc > > configuration. > > > > These methods could be always available simply generate an error for > non-GPU vector types. > > > An alternative might be to compose the various `VecCUSPGetArray` and > related > > methods with `Vec` objects. You could then query these methods and use > them > > if available (and handle absence of the methods if needed, e.g. throw an > > exception). This would be easy to add. > > > > Yes, however note that what we really need is a backend-independent > way to get the RAW pointer to the GPU buffers. Right now we have calls > to get the CUSP or ViennaCL array, to handle them you need C++ and > depend on CUDA/OpenCL at compile-time. > Why would we want this? The packages themselves (CUDA/ViennaCL) only expose memory using these specific types. What use is it to wrap these up in a void * if you just have to caste back down to use them. Isn't it better to maintain type-specific, and type safe, interfaces for this stuff? Matt > > -- > Lisandro Dalcin > ============ > Research Scientist > Computer, Electrical and Mathematical Sciences & Engineering (CEMSE) > Numerical Porous Media Center (NumPor) > King Abdullah University of Science and Technology (KAUST) > http://numpor.kaust.edu.sa/ > > 4700 King Abdullah University of Science and Technology > al-Khawarizmi Bldg (Bldg 1), Office # 4332 > Thuwal 23955-6900, Kingdom of Saudi Arabia > http://www.kaust.edu.sa > > Office Phone: +966 12 808-0459 > -- What most experimenters take for granted before they begin their experiments is infinitely more interesting than any results to which their experiments lead. -- Norbert Wiener

1 0

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

17 Oct '14

On 17 October 2014 17:54, Dominic Meiser <dmeiser(a)txcorp.com> wrote: > Hi Ashwin, > > Are you suggesting that `VecGetGPUArray*` be added to the `Vec` interface? > That might be problematic because these methods only makes sense for GPU > vectors. The interface of `Vec` would then be tied to the PETSc > configuration. > These methods could be always available simply generate an error for non-GPU vector types. > An alternative might be to compose the various `VecCUSPGetArray` and related > methods with `Vec` objects. You could then query these methods and use them > if available (and handle absence of the methods if needed, e.g. throw an > exception). This would be easy to add. > Yes, however note that what we really need is a backend-independent way to get the RAW pointer to the GPU buffers. Right now we have calls to get the CUSP or ViennaCL array, to handle them you need C++ and depend on CUDA/OpenCL at compile-time. -- Lisandro Dalcin ============ Research Scientist Computer, Electrical and Mathematical Sciences & Engineering (CEMSE) Numerical Porous Media Center (NumPor) King Abdullah University of Science and Technology (KAUST) http://numpor.kaust.edu.sa/ 4700 King Abdullah University of Science and Technology al-Khawarizmi Bldg (Bldg 1), Office # 4332 Thuwal 23955-6900, Kingdom of Saudi Arabia http://www.kaust.edu.sa Office Phone: +966 12 808-0459

1 0

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

17 Oct '14

Hi Ashwin, Are you suggesting that `VecGetGPUArray*` be added to the `Vec` interface? That might be problematic because these methods only makes sense for GPU vectors. The interface of `Vec` would then be tied to the PETSc configuration. An alternative might be to compose the various `VecCUSPGetArray` and related methods with `Vec` objects. You could then query these methods and use them if available (and handle absence of the methods if needed, e.g. throw an exception). This would be easy to add. I'm not an expert on the PETSc object model so hopefully somebody else can comment on whether this would be an idiomatic PETSc solution. Cheers, Dominic On 10/17/2014 08:21 AM, Ashwin Srinath wrote: > Hello, petsc-dev > > When working with petsc4py, I realized that there was no way to access > the underlying device memory. The routine that PETSc provides is > VecCUSPGetArrayRead/Write, which of course, makes no sense in Python. > > So I wrote a petsc4py extension that extracts and returns the raw > device pointer from the underlying CUSP array. With this raw pointer, > I'm able to construct a PyCUDA GPUArray, and apply my own kernels to > the underlying buffers. My code is available here: > https://github.com/ashwinsrnth/petsc-pycuda > > 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. > > Thank you > Ashwin -- Dominic Meiser Tech-X Corporation 5621 Arapahoe Avenue Boulder, CO 80303 USA Telephone: 303-996-2036 Fax: 303-448-7756 www.txcorp.com

1 0

Backend independent VecGetArray for GPUs
by Ashwin Srinath 17 Oct '14

17 Oct '14

Hello, petsc-dev When working with petsc4py, I realized that there was no way to access the underlying device memory. The routine that PETSc provides is VecCUSPGetArrayRead/Write, which of course, makes no sense in Python. So I wrote a petsc4py extension that extracts and returns the raw device pointer from the underlying CUSP array. With this raw pointer, I'm able to construct a PyCUDA GPUArray, and apply my own kernels to the underlying buffers. My code is available here: https://github.com/ashwinsrnth/petsc-pycuda 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. Thank you Ashwin

1 0

Re: [petsc-dev] SF broken in master !
by Barry Smith 15 Oct '14

15 Oct '14

On Oct 15, 2014, at 5:24 AM, Matthew Knepley <knepley(a)gmail.com> wrote: > On Wed, Oct 15, 2014 at 12:04 AM, Jed Brown <jed(a)jedbrown.org> wrote: > > > Git should make it easier to just push this back onto the branch so that > it cannot be forgotten. You are trying to make git a ""do what I mean” language instead of a "pain in the butt language” :-) You are right that too often git makes the “correct” way to do something too involved so people skip it. When you see the typo Jed expects you to 1) back out of what you are doing (which means remembering how to get back to the state before you started the merge) 2) change the branch, 3) make a commit of the branch 4) do the merge again This is cumbersome and annoying when all that is needed is for you to be able to announce that you want your change to belong to the branch and git could do the rest but instead you, the human, is suppose to do the manual error prone process of backing out and redoing when that is something a computer can do trivially. This happens many places in git. > > Matt > > -- > What most experimenters take for granted before they begin their experiments is infinitely more interesting than any results to which their experiments lead. > -- Norbert Wiener

1 0

Re: [petsc-dev] SF broken in master !
by Matthew Knepley 15 Oct '14

15 Oct '14

On Wed, Oct 15, 2014 at 12:04 AM, Jed Brown <jed(a)jedbrown.org> wrote: > Barry Smith <bsmith(a)mcs.anl.gov> writes: > > > and yet it is not broke in next. I thought that was impossible! > > This commit was bad: > > > https://bitbucket.org/petsc/petsc/commits/60a1948e59517edd5702a5e277d78229b… > > Toby pointed it out and fixed it in this commit, which went into 'next': > > > https://bitbucket.org/petsc/petsc/commits/38e7336fef8d310d693744ab39306ec09… > > Meanwhile, Matt didn't put this in his branch and went on to make other > changes to SF, including a similar commit: > > > https://bitbucket.org/petsc/petsc/commits/9837ea968140067d7f638ef40a3e6ee2b… > > Again, Toby pointed out the bug in this commit, which Matt had to > resolve when merging to 'next' because it conflicted with Toby's correct > version. The tests certainly did not pass in Matt's branch, but they > passed in 'next' because Toby's branch fixed the bug. > > Ultimately, Matt merged to 'master' without merging Toby's bug fix. > > I have now merged Toby's branch and 'master' works again. Thanks, Toby. > > > Note that prior to this merge, the following was the only difference > between 'master' and 'next': > > diff --git c/src/vec/is/sf/interface/sf.c w/src/vec/is/sf/interface/sf.c > index af2dc58..26a286a 100644 > --- c/src/vec/is/sf/interface/sf.c > +++ w/src/vec/is/sf/interface/sf.c > @@ -794,7 +794,7 @@ PetscErrorCode PetscSFGetMultiSF(PetscSF sf,PetscSF > *multi) > ierr = PetscMalloc1(sf->nleaves,&remote);CHKERRQ(ierr); > for (i=0; i<sf->nleaves; i++) { > remote[i].rank = sf->remote[i].rank; > - remote[i].index = outoffset[sf->mine ? sf->mine[i] : 1]; > + remote[i].index = outoffset[sf->mine ? sf->mine[i] : i]; > } > ierr = > PetscSFDuplicate(sf,PETSCSF_DUPLICATE_RANKS,&sf->multi);CHKERRQ(ierr); > ierr = > PetscSFSetGraph(sf->multi,inoffset[sf->nroots],sf->nleaves,NULL,PETSC_COPY_VALUES,remote,PETSC_OWN_POINTER);CHKERRQ(ierr); > > > To avoid this in the future, I suggest > > 1. Run tests on topic branches, especially those tests related to the > modification. If you're tinkering with SF, the least you can do is > run the SF tests. > Of course I ran the tests and they passed in next. > 2. Fix content bugs in the topic branch instead of sneaking it into a > merge commit that's oh so easy to forget. > Git should make it easier to just push this back onto the branch so that it cannot be forgotten. Matt -- What most experimenters take for granted before they begin their experiments is infinitely more interesting than any results to which their experiments lead. -- Norbert Wiener

1 0

Re: [petsc-dev] SF broken in master !
by Tobin Isaac 15 Oct '14

15 Oct '14

On Wed, Oct 15, 2014 at 12:52:24AM -0500, Tobin Isaac wrote: > On Wed, Oct 15, 2014 at 12:04:47AM -0500, Jed Brown wrote: > > Barry Smith <bsmith(a)mcs.anl.gov> writes: > > > > > and yet it is not broke in next. I thought that was impossible! > > > > This commit was bad: > > > > https://bitbucket.org/petsc/petsc/commits/60a1948e59517edd5702a5e277d78229b… > > > > Toby pointed it out and fixed it in this commit, which went into 'next': > > > > https://bitbucket.org/petsc/petsc/commits/38e7336fef8d310d693744ab39306ec09… > > > > Meanwhile, Matt didn't put this in his branch and went on to make other > > changes to SF, including a similar commit: > > > > https://bitbucket.org/petsc/petsc/commits/9837ea968140067d7f638ef40a3e6ee2b… > > > > Again, Toby pointed out the bug in this commit, which Matt had to > > resolve when merging to 'next' because it conflicted with Toby's correct > > version. The tests certainly did not pass in Matt's branch, but they > > passed in 'next' because Toby's branch fixed the bug. > > > > Ultimately, Matt merged to 'master' without merging Toby's bug fix. > > > > I have now merged Toby's branch and 'master' works again. Thanks, Toby. > > The praise might be misplaced: their bad commit assumed that sf->nroots > is always as big as the index space of the leaves; my commit assumed > that sf->nleaves was always big enough. For sparse leaves, neither > may be the case (see Michael's comment on the first commit above). I > though Matt had a commit that was the "right" think to do... This is the one: https://bitbucket.org/petsc/petsc/commits/9837ea968140067d7f638ef40a3e6ee2b… I think the actual problem is that Matt fixed a typo in the merge into next and it didn't get rerere'd into master. Toby > > Toby > > > > > > > Note that prior to this merge, the following was the only difference > > between 'master' and 'next': > > > > diff --git c/src/vec/is/sf/interface/sf.c w/src/vec/is/sf/interface/sf.c > > index af2dc58..26a286a 100644 > > --- c/src/vec/is/sf/interface/sf.c > > +++ w/src/vec/is/sf/interface/sf.c > > @@ -794,7 +794,7 @@ PetscErrorCode PetscSFGetMultiSF(PetscSF sf,PetscSF *multi) > > ierr = PetscMalloc1(sf->nleaves,&remote);CHKERRQ(ierr); > > for (i=0; i<sf->nleaves; i++) { > > remote[i].rank = sf->remote[i].rank; > > - remote[i].index = outoffset[sf->mine ? sf->mine[i] : 1]; > > + remote[i].index = outoffset[sf->mine ? sf->mine[i] : i]; > > } > > ierr = PetscSFDuplicate(sf,PETSCSF_DUPLICATE_RANKS,&sf->multi);CHKERRQ(ierr); > > ierr = PetscSFSetGraph(sf->multi,inoffset[sf->nroots],sf->nleaves,NULL,PETSC_COPY_VALUES,remote,PETSC_OWN_POINTER);CHKERRQ(ierr); > > > > > > To avoid this in the future, I suggest > > > > 1. Run tests on topic branches, especially those tests related to the > > modification. If you're tinkering with SF, the least you can do is > > run the SF tests. > > > > 2. Fix content bugs in the topic branch instead of sneaking it into a > > merge commit that's oh so easy to forget. > >

1 0

Re: [petsc-dev] SF broken in master !
by Tobin Isaac 15 Oct '14

15 Oct '14

On Wed, Oct 15, 2014 at 12:04:47AM -0500, Jed Brown wrote: > Barry Smith <bsmith(a)mcs.anl.gov> writes: > > > and yet it is not broke in next. I thought that was impossible! > > This commit was bad: > > https://bitbucket.org/petsc/petsc/commits/60a1948e59517edd5702a5e277d78229b… > > Toby pointed it out and fixed it in this commit, which went into 'next': > > https://bitbucket.org/petsc/petsc/commits/38e7336fef8d310d693744ab39306ec09… > > Meanwhile, Matt didn't put this in his branch and went on to make other > changes to SF, including a similar commit: > > https://bitbucket.org/petsc/petsc/commits/9837ea968140067d7f638ef40a3e6ee2b… > > Again, Toby pointed out the bug in this commit, which Matt had to > resolve when merging to 'next' because it conflicted with Toby's correct > version. The tests certainly did not pass in Matt's branch, but they > passed in 'next' because Toby's branch fixed the bug. > > Ultimately, Matt merged to 'master' without merging Toby's bug fix. > > I have now merged Toby's branch and 'master' works again. Thanks, Toby. The praise might be misplaced: their bad commit assumed that sf->nroots is always as big as the index space of the leaves; my commit assumed that sf->nleaves was always big enough. For sparse leaves, neither may be the case (see Michael's comment on the first commit above). I though Matt had a commit that was the "right" think to do... Toby > > > Note that prior to this merge, the following was the only difference > between 'master' and 'next': > > diff --git c/src/vec/is/sf/interface/sf.c w/src/vec/is/sf/interface/sf.c > index af2dc58..26a286a 100644 > --- c/src/vec/is/sf/interface/sf.c > +++ w/src/vec/is/sf/interface/sf.c > @@ -794,7 +794,7 @@ PetscErrorCode PetscSFGetMultiSF(PetscSF sf,PetscSF *multi) > ierr = PetscMalloc1(sf->nleaves,&remote);CHKERRQ(ierr); > for (i=0; i<sf->nleaves; i++) { > remote[i].rank = sf->remote[i].rank; > - remote[i].index = outoffset[sf->mine ? sf->mine[i] : 1]; > + remote[i].index = outoffset[sf->mine ? sf->mine[i] : i]; > } > ierr = PetscSFDuplicate(sf,PETSCSF_DUPLICATE_RANKS,&sf->multi);CHKERRQ(ierr); > ierr = PetscSFSetGraph(sf->multi,inoffset[sf->nroots],sf->nleaves,NULL,PETSC_COPY_VALUES,remote,PETSC_OWN_POINTER);CHKERRQ(ierr); > > > To avoid this in the future, I suggest > > 1. Run tests on topic branches, especially those tests related to the > modification. If you're tinkering with SF, the least you can do is > run the SF tests. > > 2. Fix content bugs in the topic branch instead of sneaking it into a > merge commit that's oh so easy to forget.

1 0

Re: [petsc-dev] SF broken in master !
by Jed Brown 15 Oct '14

15 Oct '14

Barry Smith <bsmith(a)mcs.anl.gov> writes: > and yet it is not broke in next. I thought that was impossible! This commit was bad: https://bitbucket.org/petsc/petsc/commits/60a1948e59517edd5702a5e277d78229b… Toby pointed it out and fixed it in this commit, which went into 'next': https://bitbucket.org/petsc/petsc/commits/38e7336fef8d310d693744ab39306ec09… Meanwhile, Matt didn't put this in his branch and went on to make other changes to SF, including a similar commit: https://bitbucket.org/petsc/petsc/commits/9837ea968140067d7f638ef40a3e6ee2b… Again, Toby pointed out the bug in this commit, which Matt had to resolve when merging to 'next' because it conflicted with Toby's correct version. The tests certainly did not pass in Matt's branch, but they passed in 'next' because Toby's branch fixed the bug. Ultimately, Matt merged to 'master' without merging Toby's bug fix. I have now merged Toby's branch and 'master' works again. Thanks, Toby. Note that prior to this merge, the following was the only difference between 'master' and 'next': diff --git c/src/vec/is/sf/interface/sf.c w/src/vec/is/sf/interface/sf.c index af2dc58..26a286a 100644 --- c/src/vec/is/sf/interface/sf.c +++ w/src/vec/is/sf/interface/sf.c @@ -794,7 +794,7 @@ PetscErrorCode PetscSFGetMultiSF(PetscSF sf,PetscSF *multi) ierr = PetscMalloc1(sf->nleaves,&remote);CHKERRQ(ierr); for (i=0; i<sf->nleaves; i++) { remote[i].rank = sf->remote[i].rank; - remote[i].index = outoffset[sf->mine ? sf->mine[i] : 1]; + remote[i].index = outoffset[sf->mine ? sf->mine[i] : i]; } ierr = PetscSFDuplicate(sf,PETSCSF_DUPLICATE_RANKS,&sf->multi);CHKERRQ(ierr); ierr = PetscSFSetGraph(sf->multi,inoffset[sf->nroots],sf->nleaves,NULL,PETSC_COPY_VALUES,remote,PETSC_OWN_POINTER);CHKERRQ(ierr); To avoid this in the future, I suggest 1. Run tests on topic branches, especially those tests related to the modification. If you're tinkering with SF, the least you can do is run the SF tests. 2. Fix content bugs in the topic branch instead of sneaking it into a merge commit that's oh so easy to forget.

1 0