On Feb 6, 2012, at 2:46 PM, Dmitry Karpeev wrote: > > > On Mon, Feb 6, 2012 at 2:33 PM, Barry Smith <bsmith at mcs.anl.gov> wrote: > > On Feb 6, 2012, at 2:28 PM, Dmitry Karpeev wrote: > > > > > > > On Mon, Feb 6, 2012 at 2:06 PM, Barry Smith <bsmith at mcs.anl.gov> wrote: > > > > On Feb 6, 2012, at 2:02 PM, Dmitry Karpeev wrote: > > > > > > > > > > > On Mon, Feb 6, 2012 at 1:50 PM, Barry Smith <bsmith at mcs.anl.gov> wrote: > > > > > > On Feb 6, 2012, at 1:42 PM, Dmitry Karpeev wrote: > > > > > > > > > > > > > > > On Mon, Feb 6, 2012 at 1:30 PM, Barry Smith <bsmith at mcs.anl.gov> > > > > wrote: > > > > > > > > On Feb 6, 2012, at 1:27 PM, Matthew Knepley wrote: > > > > > > > > > On Mon, Feb 6, 2012 at 1:23 PM, Barry Smith <bsmith at mcs.anl.gov> > > > > > wrote: > > > > > > > > > > On Feb 6, 2012, at 1:14 PM, Matthew Knepley wrote: > > > > > > > > > > > On Mon, Feb 6, 2012 at 1:11 PM, Barry Smith <bsmith at mcs.anl.gov> > > > > > > wrote: > > > > > > > > > > > > On Feb 6, 2012, at 12:47 PM, Jed Brown wrote: > > > > > > > > > > > > > On Mon, Feb 6, 2012 at 21:42, Matthew Knepley <knepley at > > > > > > > gmail.com> wrote: > > > > > > > I don't like this because it would mean calling VecSetUp() all > > > > > > > over the place. Couldn't the ghosting flag be on the same > > > > > > > level as the sizes? > > > > > > > > > > > > > > Maybe VecSetUp() is wrong because that would imply collective. > > > > > > > This memory allocation is simple and need not be collective. > > > > > > > > > > > > > > Ghosting information is an array, so placing it in VecSetSizes() > > > > > > > would seem unnatural to me. I wouldn't really want > > > > > > > VecSetGhosts(Vec,PetscInt,const PetscInt*) to be order-dependent > > > > > > > with respect to VecSetType(), but maybe the VecSetUp() would be > > > > > > > too messy. > > > > > > > > > > > > Only some vectors support ghosting, so the usual PETSc way (like > > > > > > with KSPGMRESRestart()) is to calling the specific setting routines > > > > > > ONLY AFTER the type has been set. Otherwise all kinds of oddball > > > > > > type specific stuff needs to be cached in the object and then > > > > > > pulled out later; possible but is that desirable? Who decides what > > > > > > can be set before the type and what can be set after? Having a > > > > > > single rule, anything appropriate for a subset of the types must be > > > > > > set after the type is set is a nice simple model. > > > > > > > > > > > > On the other hand you could argue that ALL vector types should > > > > > > support ghosting as a natural thing (with sequential vectors just > > > > > > have 0 length ghosts conceptually) then it would be desirable to > > > > > > allow setting the ghost information in any ordering. > > > > > > > > > > > > I will argue this. > > > > > > > > > > Ok, then just like VecSetSizes() we stash this information if given > > > > > before the type is set and use it when the type is set. However if > > > > > it is set after the type is set (and after the sizes are set) then we > > > > > need to destroy the old datastructure and build a new one which means > > > > > messier code. By instead actually allocating the data structure at > > > > > VecSetUp() the code is cleaner because we never need to take down and > > > > > rebuild a data structure and yet order doesn't matter. Users WILL > > > > > need to call VecSetUp() before VecSetValues() and possibly a few > > > > > other things like they do with Mat now. > > > > > > > > > > We just disallow setting it after the type, just like sizes. I don't > > > > > see the argument against this. > > > > > > > > We allow setting the sizes after the type. > > > > > > > > Since we are on a related subject: should then all type-specific > > > > processing of sizes be moved out of MatSetSizes() > > > > into MatSetUp? By this I mean this code: > > > > if (A->ops->setsizes) { > > > > /* Since this will not be set until the type has been set, this > > > > will NOT be called on the initial > > > > call of MatSetSizes() (which must be called BEFORE MatSetType() > > > > */ > > > > ierr = (*A->ops->setsizes)(A,m,n,M,N);CHKERRQ(ierr); > > > > } else { > > > > > > > > This eliminates the need to check for the presence of various > > > > type-specific setup methods -- they will all be called in MatSetUp > > > > after the type is guaranteed to have been set. This would also make > > > > MatSetSizes not collective. I imagine that Vec could be organized the > > > > same way. I actually would prefer VecSetUp to explicitly delineate the > > > > end of the "factory" phase. > > > > > > The only Mat one is MatSetSizes_SeqDense() which could be easily nuked > > > (someone please check my reasoning) and then the whole concept of > > > (*setsizes)() be removed for Mat? > > > > > > I think it's useful to allow each Mat type to process size information > > > its own way. > > > For example "graph-oriented" types like MATADJ, MATIJ may want to relax > > > some constraints on the column index spaces (since they are > > > inconsequential when MatMult isn't implemented). > > > > > > Dmitry, > > > > I am confused, I though you just argued it could be removed, now you > > want to keep it? Do you want me to remove the MatSetSizes_*() stuff or not? > > > > > > My understanding is that you were stating we could remove the > > MatSetSizes_*() and just have that work handled by the MatSetUp_*() or the > > MatXXXSetPreallocation()? > > > > I'm arguing, in part, that MatSetSizes should be handled by a type-specific > > method. I guess it didn't really occur to me that it could be handled by > > MatSetup_XXX or MatXXXSetPreallocation directly rather than by > > MatSetValues_XXX, but this is as good an arrangement as any. This is more > > or less the situation now, except that MatSetSizes_XXX could get > > eliminated, so this isn't much of an argument. > > > > What I'm *also* saying is that the logic for checking whether it's safe to > > invoke a type-specific method or not is scattered around (e.g., in > > MatSetSizes()). Additionally, because of these conditionals in the code > > these methods sometimes are collective (e.g., MatSetSizes() is collective, > > when the type has been set and we are indeed going to process the sizes, > > set up PetscLayouts, etc.), or not (e.g., when the type isn't set and > > MatSetSizes() merely stashes the sizes until better times). Making methods > > like MatSetSizes() only stash the arguments and then process them all in > > MatSetUp would eliminate the scattered-around conditionals and remove the > > ambiguity about collectivity: things like MatSetSizes aren't collective, > > but MatSetUp is. > > > > I think this is more or less is what is being proposed for Vec as well, > > isn't it? This also more cleanly reflects the "factory" paradigm, which, > > it seems to me, implicitly underlies the construction of many PETSc classes. > > The original goal of the (now) convoluted code was to avoid the need for > VecSetUp() and MatSetUp() and avoid the need (when at all possible) for > stashing type specific information for later use; the code became more > convoluted in an attempt to keep the no setup paradigm. > > Okay, so it seems like we are all for the stashing and an explicit call > VecSetUp or MatSetUp to process the stashed data later. I'm definitely for > it.
Wow, hold on. How much stuff stashed? Anything? Or just sizes and ghost points. Stash the preallocation information and don't preallocate until the setup? If the user sets five different preallocation informations do we stash them all and us the right one at setup? Currently MatXXXSetPreallocation() is ignored unless the type as already been set to XXX and then it immediately builds the data structure at that point, it does not stash it until MatSetUp() Barry > > Dmitry. > > Barry > > > > > Dmitry. > > > > Barry > > > > > > > > Dmitry. > > > > > > > > > Barry > > > > > > > > > > > Dmitry. > > > > > > > > > > > > > > > > > > Matt > > > > > > > > > > > > > > > Barry > > > > > > > > > > > > > > > > > Sadly we now pretty much require MatSetUp() or a > > > > > > MatXXXSetPreallocation() to be called so why not always have > > > > > > VecSetUp() always called? > > > > > > > > > > > > Because I don't think we need it and it is snother layer of > > > > > > complication for the user and us. I think > > > > > > we could make it work where it was called automatically when > > > > > > necessary, but that adds another > > > > > > headache for maintenance and extension. > > > > > > > > > > > > Matt > > > > > > > > > > > > We have not converged yet, > > > > > > > > > > > > Barry > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > > 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 > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > > 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 > > > > > > > > > > > > > > > > > > > >