Re: [petsc-dev] DMDA_*PERIODIC and DMDA_XYZGHOSTED
On Mar 2, 2011, at 6:11 PM, Ethan Coon wrote:
On Wed, 2011-03-02 at 16:26 -0600, Barry Smith wrote:
On Mar 2, 2011, at 1:47 PM, Ethan Coon wrote:
Ethan,
MatSetValues() and VecSetValues() handle negative indices as "ignore these entries". Currently VecScatterCreate() does not handle negative indices as "ignore these entries" (at least it is not documented and I did not write it), likely it will either crash or generate an error.
It sounds like you are proposing that if the from or to entry in a particular "slot" is negative you would like VecScatterCreate() to just ignore that slot? This seems like an ok proposal if you are willing to update VecScatterCreate() to handle it and add to VecScatterCreate() manual page this feature. If this truly simplifies all the horrible if () code in the DA construction to handle corner stuff then it would be worth doing.
Hmm, I was proposing that, but because I thought that was the case already.
It would clean up the DASetUp code, but not as much as I thought initially. Currently VecSetValuesLocal(), when used with the L2G mapping from a STAR_STENCIL DMDA, will happily add/insert values from the ghost cells in the corner to the global vector (why you would add values to a ghost node on which you don't want to get information back from I don't know, but someone made an effort to implement it...).
I think that is a "bug" or "unintended feature" I hope nobody worked hard to get it to work. I think it is just that way because that is the way it worked out. Probably DMGetMatrix_DA() should call MatSetOption(mat,MAT_NO_NEW_NONZEROS,PETSC_TRUE); to prevent people of accidently using those slots (if they really want to for some perverse reason they could call MatSetOption() themselves to reset it.
Barry
To keep that feature, the L2GMapping must be different from the IS used for the DMDAGlobalToLocal scatter, and all the ugly if crap has to stay.
Even in the star case the L2G has to contain slots for those stencil points (though you could fill those slots with negative entries I guess) to get the VecSetValuesLocal() to work. Is that what you propose, putting negative numbers there?? Seems possibly ok to me. The one danger is that if they user intends to set that corner value with VecSetValuesLocal() or MatSetValuesLocal() it will be silently discarded (of course they should never try to set it but they may).
Barry
Right, got it, there has to be local number for the gxs,gys,gzs entry even in the star case. The question is if it has to get mapped someplace.
Using VecSetValuesLocal() on an entire local domain with ADD_VALUES, the current implementation will sum the entries from ghost nodes and interior nodes, while in the INSERT_VALUES, it would lead to a race condition, where the processor who owns the value may not win (and it does so silently). (Note this is true for any stencil and any ghost node, not just corners in the star stencil.)
Using INSERT_VALUES with VecSetValues() also as well as the MatSetValues versions always has the condition that it is undefined who wins when several processes try to put in the same location. This is just a fact of (PETSc) life. I don't think DM or SetValuesLocal() really makes it any worse.
If instead we put -1 in all ghost indices of the l2g mapping, then VecSetValuesLocal() with INSERT_VALUES functions as expected (the value in the global vec is the value given by the process owning the node), while ADD_VALUES will only add in the value given by the process owning the node (and do so silently). This behavior for ADD_VALUES is exactly what is described in the "notes" section of DMLocalToGlobalBegin's man page. Basically it just doesn't allow you to VecSetLocalValues() on ghost nodes.
Compare this to the result of the operation: DMDAVecGetArray(da, local), assignment to the array, restore the array, then call DMDALocalToGlobal(). With DMDALocalToGlobal() and INSERT_VALUES, it does the l2g scatter, and so there is no race condition and the global vec gets the value in the array of the process owning that entry. With ADD_VALUES, it does the reverse of the g2l scatter, so all values get added in.
I guess I'm more concerned about the undocumented race condition than not adding values from ghost cells, which could easily be explained in a man page. So yes, I'm proposing to put -1 as the global index of all ghost nodes in the local to global mapping.
Note that I have to put something in the ghosted (nonperiodic) local number spots as well -- they have no corresponding global number, so it has to be -1. At least this is then consistent that all ghost nodes in a VecSetValuesLocal() get ignored. And if you really want to do this, it's likely you're using the (safer) DMLocalToGlobal() way anyway.
Go ahead and add support for VecScatterCreate() to handle negative indices and simplify (greatly) the DACreates if you are up for it. Thanks Barry
Ethan
I can just graft the Ghosted case on to that code (making it only slightly more ugly). It will still depend upon the VecSetValues() accepting and ignoring negative global indices, but that's already the case.
Ethan
Performance is not an issue since you would just discard those slots in the VecScatterCreate() phase and they would never appear in the actual scatter operations.
Barry
Ethan
On Tue, 2010-12-07 at 15:41 -0700, Ethan Coon wrote:
On Tue, 2010-12-07 at 13:45 -0600, Barry Smith wrote:
DMDA_XYZGHOSTED does not exist for 2d and 3d it was added, I'm guessing, as an experiment and was never in the initial design of DMDA. To fully support it one needs to go back tot he design of DMDA and see how to have it properly done and not just bolt it on. Some people like to use these types of ghost nodes so I agree it is a useful thing to have but who is going to properly add it?
At some point in the not-too-distant future I'll get frustrated enough to look into this, but I don't have the time at the moment. At first glance it looks like:
- Ensure DMDA{X,Y,Z}Periodic() macros are used everywhere instead of direct comparisons to dd->wrap (they aren't used everywhere currently).
- Define macros DMDA{X,Y,Z}Ghosted() to (in some places) replace DMDA{X,Y,Z}Periodic() and then choosing the appropriate macro in the right places.
- This probably doesn't merit a change in the DMDACreate* API (it would affect a very large amount of user code). The most obvious alternative to an API change would be a larger, somewhat convoluted enum for the PeriodicType (DMDA_XPERIODIC_YGHOSTED, DMDA_XYGHOSTED, etc) which could at least be made backward compatible.
At least all of the functionality should be there already (since it's needed in the periodic case)... it's just higher level code that would need to change.
Ethan
On Dec 7, 2010, at 1:30 PM, Jed Brown wrote:
On Tue, Dec 7, 2010 at 20:21, Ethan Coon <[email protected]> wrote: 'd like a DA where there are ghost cells on every boundary, and some of those ghost cells (but not all) are filled in with periodic values.
It would be useful to people doing explicit stuff if there was a way to get ghost nodes in the local vector without implying periodic communication (and weird coordinate management).
A related issue for purely explicit is to have a way to VecAXPY without needing to copy to and from a global vector. (TSSSP has low-memory schemes, paying for an extra vector or two is actually significant in that context, and (less significant) I'm certain I can cook up a realistic benchmark where the memcpy costs more than 10%.) I think I know how to implement this sharing transparently (more-or-less using VecNest) so we could make it non-default but be able to activate it at runtime.
Why can you not use VecAXPY() on the local Vecs?
Barry
Jed
-- ------------------------------------ Ethan Coon Post-Doctoral Researcher Applied Mathematics - T-5 Los Alamos National Laboratory 505-665-8289
http://www.ldeo.columbia.edu/~ecoon/ ------------------------------------
-- ------------------------------------ Ethan Coon Post-Doctoral Researcher Applied Mathematics - T-5 Los Alamos National Laboratory 505-665-8289
http://www.ldeo.columbia.edu/~ecoon/ ------------------------------------
-- ------------------------------------ Ethan Coon Post-Doctoral Researcher Applied Mathematics - T-5 Los Alamos National Laboratory 505-665-8289
http://www.ldeo.columbia.edu/~ecoon/ ------------------------------------
participants (1)
-
Barry Smith