February 2013 - petsc-dev - mailman3.cels.anl.gov

Re: [petsc-dev] concurrencykit
by Jed Brown 19 Feb '13

19 Feb '13

On Tue, Feb 19, 2013 at 3:51 PM, Barry Smith <bsmith(a)mcs.anl.gov> wrote: > I can live with it. You, of course, are providing a --download-ck before > writing a single line of code that uses ck. Yes, of course I wrote that first.

1 0

Re: [petsc-dev] concurrencykit
by Barry Smith 19 Feb '13

19 Feb '13

On Feb 19, 2013, at 3:43 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > I've started adding support for using this library for memory primitives and some basic lock-free data structures. > > http://concurrencykit.org/ > > Of course when threads are not used, we need some replacement. All the memory barriers and assignments become trivial and locks disappear so the no-threading variant is trivial to implement. My question now is whether to call ck_* functions directly from PETSc source or to always wrap them in Petsc* namespaced macros and inline functions. > > I would normally always use a Petsc namespace for this kind of thing, but this library has convenient man pages and is, I think, the best thing out there for implementing these primitives. Wrapping everything in a Petsc* namespace would thus make it less direct to get down to the actual semantics (or a man page). Thus I have half a mind to treat it like MPI and just provide non-threaded implementations in petscck.h when !defined(PETSC_HAVE_CONCURRENCYKIT). Is this a bad idea? I can live with it. You, of course, are providing a --download-ck before writing a single line of code that uses ck. Barry

1 0

concurrencykit
by Jed Brown 19 Feb '13

19 Feb '13

I've started adding support for using this library for memory primitives and some basic lock-free data structures. http://concurrencykit.org/ Of course when threads are not used, we need some replacement. All the memory barriers and assignments become trivial and locks disappear so the no-threading variant is trivial to implement. My question now is whether to call ck_* functions directly from PETSc source or to always wrap them in Petsc* namespaced macros and inline functions. I would normally always use a Petsc namespace for this kind of thing, but this library has convenient man pages and is, I think, the best thing out there for implementing these primitives. Wrapping everything in a Petsc* namespace would thus make it less direct to get down to the actual semantics (or a man page). Thus I have half a mind to treat it like MPI and just provide non-threaded implementations in petscck.h when !defined(PETSC_HAVE_CONCURRENCYKIT). Is this a bad idea?

1 0

Re: [petsc-dev] Error with ML preconditioner
by Jed Brown 18 Feb '13

18 Feb '13

On Mon, Feb 18, 2013 at 4:04 PM, Mark F. Adams <mark.adams(a)columbia.edu>wrote: > Yes it should be removed, I just deleted the whole section and tested it. > > We could reinstate this code and check that all processors have a blocked > index set before making blocked ISs. > I don't want to add another MPI_Allreduce to the critical path (executed on each assembly). So we either make it strict enough to error if you declare a block size and don't fill in the blocks, we always fill in the blocks for you (maybe costing memory), we make the VecScatter permissive enough to have padded blocks, or we never optimize for the blocks. To answer this, we have to decide how much the block size attribute is about the _interface_ (MatSetValuesBlocked) and how much is about data structure optimizations.

1 0

Re: [petsc-dev] Error with ML preconditioner
by Garth N. Wells 18 Feb '13

18 Feb '13

On 18 February 2013 21:36, Mark F. Adams <mark.adams(a)columbia.edu> wrote: > > On Feb 18, 2013, at 4:00 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > > Hmm, this debugging check was added by Mark here. > > https://bitbucket.org/petsc/petsc-dev/commits/fb7e4343253b73d3fb40857991a8c… > > > The logic here is messed up. Valid input could result in a non-collectiv > result. So this DEBUG code should be a real check here. I think the logic > is OK to detect a blocked garray locally … but some processors could be > proper and others not as Jed pointed out. > > Barry touched the code a bit earlier to add this case: > > https://bitbucket.org/petsc/petsc-dev/commits/c997baff1558fd165412be034e812… > > Prior to that, Mark had reversed the meaning of that bit of code: > > https://bitbucket.org/petsc/petsc-dev/commits/a85625d104df12863d7d1735f9fdb… > > Even in Barry's original code, I don't understand why we can expect this > check to be producing a collective result. Surely there can be some ranks > that see everything evenly divide and others that do not. > > It seems to me that we should either (a) explicitly disallow matrices with a > block size to be built when the blocks are not filled or (b) when the user > requests blocks, pad blocks so that we can always use an ISBlock. > > > It looks like we have (a) now in a debug build. I'm not wild about (b), > this is only used to use blocked IS for efficiency and we have never measure > that this is useful. > > So I will comment out the whole 'useblockis' business and let Garth move > along. We could do an all reduce to see if everyone is blocked and then, > and only then, use th blocked ISs. This would cost an alreduce. Not sure > what would be better, but for now we can just disable this. > Thanks. Works now. Garth > > On Mon, Feb 18, 2013 at 2:35 PM, Garth N. Wells <gnw20(a)cam.ac.uk> wrote: >> >> I have some code using ML as a preconditioner, and it works fines >> with PETSc 3.3 but crashes with PETSc dev. I'm not setting any of the >> ML parameters. Hopefully someone will recognise the error (below) and >> save me having to bisect the changesets, which is hard because changes >> in PETSc dev make it hard for me to compile against earlier PETSc dev >> revisions. >> >> Garth >> >> >> [0]PETSC ERROR: --------------------- Error Message >> ------------------------------------ >> [0]PETSC ERROR: Petsc has generated inconsistent data! >> [0]PETSC ERROR: Use of blocked not consistant (I am usning blocked)! >> [0]PETSC ERROR: >> ------------------------------------------------------------------------ >> [0]PETSC ERROR: Petsc Development HG revision: >> 96a09d6a701823d69aede247ed949c1dd4e7c8ff HG Date: Mon Feb 18 11:08:29 >> 2013 -0600 >> [0]PETSC ERROR: See docs/changes/index.html for recent updates. >> [0]PETSC ERROR: See docs/faq.html for hints about trouble shooting. >> [0]PETSC ERROR: See docs/index.html for manual pages. >> [0]PETSC ERROR: >> ------------------------------------------------------------------------ >> [0]PETSC ERROR: Unknown Name on a arch-linux2-c-debug named ....... >> Mon Feb 18 19:25:17 2013 >> [0]PETSC ERROR: Libraries linked from /home/garth/local/gcc/petsc-dev/lib >> [0]PETSC ERROR: >> [3]PETSC ERROR: See docs/index.html for manual pages. > > >

1 0

Re: [petsc-dev] Error with ML preconditioner
by Barry Smith 18 Feb '13

18 Feb '13

I'm fine with just deleting it. On Feb 18, 2013, at 3:44 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > > On Mon, Feb 18, 2013 at 3:36 PM, Mark F. Adams <mark.adams(a)columbia.edu> wrote: > The logic here is messed up. Valid input could result in a non-collectiv result. So this DEBUG code should be a real check here. I think the logic is OK to detect a blocked garray locally … but some processors could be proper and others not as Jed pointed out. > > We either need to do the reduction always or make the normal code path safe always. >> It seems to me that we should either (a) explicitly disallow matrices with a block size to be built when the blocks are not filled or (b) when the user requests blocks, pad blocks so that we can always use an ISBlock. >> > > It looks like we have (a) now in a debug build. > > Well, only indirectly because this check will fire. We still have (a) in optimized mode in the sense that some procs will call ISCreateGeneral while others call ISCreateBlock, leading to a crash or deadlock. > > I'm not wild about (b), this is only used to use blocked IS for efficiency and we have never measure that this is useful. > > I don't know if Barry has measurements from when he originally added the feature > > https://bitbucket.org/petsc/petsc-dev/commits/4442702a5bc776781b96301ae6b15… > > > So I will comment out the whole 'useblockis' business and let Garth move along. We could do an all reduce to see if everyone is blocked and then, and only then, use th blocked ISs. This would cost an alreduce. Not sure what would be better, but for now we can just disable this. > > This is clearly wrong in the release too. I'm inclined to delete the entire useblockis code path since it seems like it will need to be implemented completely differently (if it matters for performance).

1 0

Re: [petsc-dev] Error with ML preconditioner
by Mark F. Adams 18 Feb '13

18 Feb '13

> > This is clearly wrong in the release too. I'm inclined to delete the entire useblockis code path since it seems like it will need to be implemented completely differently (if it matters for performance). Yes it should be removed, I just deleted the whole section and tested it. We could reinstate this code and check that all processors have a blocked index set before making blocked ISs.

1 0

Re: [petsc-dev] Error with ML preconditioner
by Jed Brown 18 Feb '13

18 Feb '13

On Mon, Feb 18, 2013 at 3:36 PM, Mark F. Adams <mark.adams(a)columbia.edu>wrote: > The logic here is messed up. Valid input could result in a non-collectiv > result. So this DEBUG code should be a real check here. I think the logic > is OK to detect a blocked garray locally … but some processors could be > proper and others not as Jed pointed out. > We either need to do the reduction always or make the normal code path safe always. > It seems to me that we should either (a) explicitly disallow matrices with > a block size to be built when the blocks are not filled or (b) when the > user requests blocks, pad blocks so that we can always use an ISBlock. > > > It looks like we have (a) now in a debug build. > Well, only indirectly because this check will fire. We still have (a) in optimized mode in the sense that some procs will call ISCreateGeneral while others call ISCreateBlock, leading to a crash or deadlock. > I'm not wild about (b), this is only used to use blocked IS for > efficiency and we have never measure that this is useful. > I don't know if Barry has measurements from when he originally added the feature https://bitbucket.org/petsc/petsc-dev/commits/4442702a5bc776781b96301ae6b15… > > So I will comment out the whole 'useblockis' business and let Garth move > along. We could do an all reduce to see if everyone is blocked and then, > and only then, use th blocked ISs. This would cost an alreduce. Not sure > what would be better, but for now we can just disable this. > This is clearly wrong in the release too. I'm inclined to delete the entire useblockis code path since it seems like it will need to be implemented completely differently (if it matters for performance).

1 0

Re: [petsc-dev] Error with ML preconditioner
by Mark F. Adams 18 Feb '13

18 Feb '13

On Feb 18, 2013, at 4:00 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > Hmm, this debugging check was added by Mark here. > > https://bitbucket.org/petsc/petsc-dev/commits/fb7e4343253b73d3fb40857991a8c… > The logic here is messed up. Valid input could result in a non-collectiv result. So this DEBUG code should be a real check here. I think the logic is OK to detect a blocked garray locally … but some processors could be proper and others not as Jed pointed out. > Barry touched the code a bit earlier to add this case: > > https://bitbucket.org/petsc/petsc-dev/commits/c997baff1558fd165412be034e812… > > Prior to that, Mark had reversed the meaning of that bit of code: > > https://bitbucket.org/petsc/petsc-dev/commits/a85625d104df12863d7d1735f9fdb… > > Even in Barry's original code, I don't understand why we can expect this check to be producing a collective result. Surely there can be some ranks that see everything evenly divide and others that do not. > > It seems to me that we should either (a) explicitly disallow matrices with a block size to be built when the blocks are not filled or (b) when the user requests blocks, pad blocks so that we can always use an ISBlock. > It looks like we have (a) now in a debug build. I'm not wild about (b), this is only used to use blocked IS for efficiency and we have never measure that this is useful. So I will comment out the whole 'useblockis' business and let Garth move along. We could do an all reduce to see if everyone is blocked and then, and only then, use th blocked ISs. This would cost an alreduce. Not sure what would be better, but for now we can just disable this. > > On Mon, Feb 18, 2013 at 2:35 PM, Garth N. Wells <gnw20(a)cam.ac.uk> wrote: > I have some code using ML as a preconditioner, and it works fines > with PETSc 3.3 but crashes with PETSc dev. I'm not setting any of the > ML parameters. Hopefully someone will recognise the error (below) and > save me having to bisect the changesets, which is hard because changes > in PETSc dev make it hard for me to compile against earlier PETSc dev > revisions. > > Garth > > > [0]PETSC ERROR: --------------------- Error Message > ------------------------------------ > [0]PETSC ERROR: Petsc has generated inconsistent data! > [0]PETSC ERROR: Use of blocked not consistant (I am usning blocked)! > [0]PETSC ERROR: > ------------------------------------------------------------------------ > [0]PETSC ERROR: Petsc Development HG revision: > 96a09d6a701823d69aede247ed949c1dd4e7c8ff HG Date: Mon Feb 18 11:08:29 > 2013 -0600 > [0]PETSC ERROR: See docs/changes/index.html for recent updates. > [0]PETSC ERROR: See docs/faq.html for hints about trouble shooting. > [0]PETSC ERROR: See docs/index.html for manual pages. > [0]PETSC ERROR: > ------------------------------------------------------------------------ > [0]PETSC ERROR: Unknown Name on a arch-linux2-c-debug named ....... > Mon Feb 18 19:25:17 2013 > [0]PETSC ERROR: Libraries linked from /home/garth/local/gcc/petsc-dev/lib > [0]PETSC ERROR: > [3]PETSC ERROR: See docs/index.html for manual pages. >

1 0

Re: [petsc-dev] Error with ML preconditioner
by Jed Brown 18 Feb '13

18 Feb '13

On Mon, Feb 18, 2013 at 3:00 PM, Jed Brown <jedbrown(a)mcs.anl.gov> wrote: > > Even in Barry's original code, I don't understand why we can expect this > check to be producing a collective result. Surely there can be some ranks > that see everything evenly divide and others that do not. > > It seems to me that we should either (a) explicitly disallow matrices with > a block size to be built when the blocks are not filled or (b) when the > user requests blocks, pad blocks so that we can always use an ISBlock. > It shouldn't be hard to build a test that will expose this problem. Just declare a block size (2), then assemble a matrix that does not fill out the block size. In the off-diagonal part of rank 0, we should have one full block, while in the off-diagonal part of rank 1, we should have anything that isn't a perfect block.

1 0