Sounds like a fine solution
Barry > On Jul 15, 2019, at 3:09 AM, Patrick Sanan <[email protected]> wrote: > > This continues to be annoying/confusing to users, so here's a current idea to > fix: > > When you call DMLocalToGlobal with INSERT_VALUES, the library needs to choose > which local dof get inserted, when multiple local dof map to a global dof. > Usually this is done by ignoring off-process communication, leaving an > injective map. That doesn't work for a single-rank-periodic dimension with a > non-zero stencil width. So, I propose to add logic to > DMLocalToGlobal{Begin,End}_Stag() which checks to see if a special > ltog_injective map has been defined, and if so, uses it. During > DMSetUp_Stag(), this map is only created in the problematic case. > > Pros: Only affects DMStag. Doesn't affect the API and behaves as users expect. > Cons: Have to store another scatter. A little bit more logic to maintain. > > Am Di., 18. Juni 2019 um 21:44 Uhr schrieb Smith, Barry F. > <[email protected]>: > > Ah > > > > On Jun 18, 2019, at 2:24 PM, Patrick Sanan <[email protected]> wrote: > > > > It's for domains of any width on one processor + periodic boundary > > conditions in a given direction. > > > > Am Di., 18. Juni 2019 um 21:19 Uhr schrieb Smith, Barry F. > > <[email protected]>: > > > > Is it only for super skinny domains when periodic or any size domain when > > periodic? If only super skinny you can still develop and run with one > > processor right, just not super skinny. > > > > I don't think it is worth the time and extra code but that's, of course, > > up to you. > > > > Barry > > > > DMDA actually had an entire other scatter for doing the local to global > > that worked for all cases but I threw it away a few years ago figuring the > > small failed special case as not important enough to keep around all that > > extra infrastructure. > > > > > > > On Jun 18, 2019, at 2:05 PM, Patrick Sanan <[email protected]> > > > wrote: > > > > > > > > > > > > Am Di., 18. Juni 2019 um 20:30 Uhr schrieb Smith, Barry F. > > > <[email protected]>: > > > > > > Patrick, > > > > > > I understand the problem with DMDA and periodicity with only a single > > > skinny processor in one direction, local to global won't work without > > > lots of additional code. I don't care because PETSc is for big problems > > > and that problem is not big. > > > > > > Agreed, as far as performance is concerned, but I also very much value > > > being able to run and debug small problems, in serial at first, on my > > > laptop. This skinny-periodic case doesn't matter much for DMDA, because > > > you don't really need to do a lot of local->global scatters with > > > INSERT_VALUES (just access the global vector directly). The way DMStag is > > > right now, I use this to construct coordinates and to do data migration, > > > because of the regular blocking of the local vectors. > > > > > > Could you please explain to me the problem with Stag unrelated to a > > > skinny domain with one processor and periodic boundary conditions? > > > > > > Each unknown in the global vector presumably has an unknown on the > > > local vector that "lives" at the same point. For insert why can't the > > > local to global just use the value from that location? Meanwhile for > > > ADD_VALUES why can't you just use the reverse of the global to local? > > > I don't think there's a way to know (from the point of view of the > > > VecScatter) which of several local unknowns is the right one to map to a > > > given global unknown. Usually you saying "it's the one coming from this > > > rank", but that breaks down for the skinny periodic case. > > > > > > > > > If the problem is just with skinny domain periodic one process in > > > that direction just generate the error and remove any test cases for that > > > situation. It seems absurd to me to waste time worrying about a totally > > > irrelevant special case and introducing all kinds of special constructs > > > for it is just making a mountain out of a mole hill. > > > Potential users have noticed and complained, so I need to fix it somehow > > > : ( Sounds like playing with higher-level DM stuff isn't worth the churn, > > > so I should focus on solutions internal to DMStag. > > > > > > > > > Barry > > > > > > > > > > On Jun 18, 2019, at 10:51 AM, Patrick Sanan via petsc-dev > > > > <[email protected]> wrote: > > > > > > > > Sorry about the delay on this - I'm still trying to decide on what's an > > > > elegant solution. I'll attempt to describe the issue and potential > > > > solutions, in case anyone has interest/ideas: > > > > > > > > My understanding of the way things work now > > > > - DMLocalToGlobal() sends data from "local" vectors to "global" > > > > vectors. > > > > - This "l2g" map is in general surjective (onto) but not injective > > > > (into) because local subdomains can overlap. > > > > - When DMLocalToGlobal() is called with INSERT_VALUES, there needs to > > > > be some mechanism to resolve this non-injectivity. > > > > - The mechanism is to ignore the parts of the l2g map that aren't > > > > rank-local. > > > > - This usually works very well because "local" vectors can usually be > > > > constructed to consist of degrees of freedom consisting of rank-local > > > > dof in the global representation (I like to call these "native"), plus > > > > additional dof corresponding to other dof, which are either on other > > > > ranks or not part of the global representation at all (ghosted > > > > boundaries). > > > > - This breaks down for a DMDA with a single rank in a given coordinate > > > > direction, and periodic boundary conditions. You'll get an error trying > > > > due to the non-injectivity of the map, even after excluding off-rank > > > > communication (since the ghost region maps to native dof, which never > > > > happens otherwise) > > > > - However, this isn't an issue in practice, because with DMDA you can > > > > just operate directly on a global vector. Local-to-global inserts > > > > aren't very useful in general. > > > > > > > > Issue related to DMStag > > > > - With DMStag, the same issue occurs as with DMDA (and this is the > > > > topic of this thread). > > > > - However, operating directly on a global vector isn't currently > > > > convenient with DMStag, because global vectors aren't regularly > > > > blocked, while local vectors are. > > > > - This is perhaps a design flaw to be addressed with DMStag, because > > > > the local representation is being used for two orthogonal purposes: > > > > padding out to obtain regular blocking, and padding out to include > > > > ghost regions. > > > > > > > > Potential solutions > > > > I can think of a few avenues to pursue, including > > > > 1. Whenever this could potentially arise, pad all local vectors with > > > > the necessary 0's and use ADD_VALUES [gross] > > > > 2. Add a dedicated "padded global" representation for DMStag, and tools > > > > for working with it. [elegant in that it resolves the design flaw > > > > above, but adds quite a lot of code] > > > > > > > > but here's the idea that I'm also intrigued by: > > > > - It's an abuse of notation to use INSERT_VALUES for DMLocalToGlobal() > > > > in the first place > > > > - Rather, it'd be more meaningful to use a new InsertMode value, > > > > INSERT_VALUES_LOCAL, deprecating the use of INSERT_VALUES for > > > > DMLocalToGlobal()* > > > > - This name corresponds to SCATTER_REVERSE_LOCAL which gets called in > > > > e.g. DMLocalToGlobalBegin_DA(), or the equivalent logic in > > > > DMLocalToGlobalBegin() using PetscSection > > > > - In most cases, this works just as before, but now the error message > > > > is more accurate - you can't use INSERT_VALUES_LOCAL when the l2g map > > > > isn't injective, even when ignoring parallel communication > > > > - Add another new InsertMode value, INSERT_VALUES_INJECTIVE, which asks > > > > the DM implementation to resolve the non-injectivity of the l2g map > > > > - In most cases, this would behave just like INSERT_VALUES_LOCAL and > > > > most users wouldn't know or care > > > > - However, for the case of interest here, the implementation would be > > > > free to use its implementation-dependent data to "do the right thing" > > > > - For DMStag, the right thing would be to behave consistently with the > > > > case with 2 or more ranks in the periodic direction (ignoring dof on > > > > ghost elements). > > > > - This could be done by setting up another VecScatter during > > > > DMSetUp_Stag(), and using it with INSERT_VALUES_INJECTIVE > > > > > > > > * eventually this could become an error if the l2g map was > > > > non-injective, but probably not for a long time if ever > > > > > > > > Am Do., 13. Juni 2019 um 15:09 Uhr schrieb Patrick Sanan > > > > <[email protected]>: > > > > I will attempt to just fix the underlying issue here, that is allow > > > > DMStagSetUniformCoordinates() to work for period boundary conditions > > > > with a single rank in the corresponding dimension. > > > > > > > > Am Mi., 12. Juni 2019 um 19:10 Uhr schrieb Patrick Sanan > > > > <[email protected]>: > > > > Ah, okay, so unless something weird is going on, that probably should > > > > never have passed (if it's asking for periodic BCs with one rank in a > > > > given direction, and INSERT_VALUES, so it's not well-defined which of > > > > the multiple local dof mapping to a given global dof should be used). > > > > I'll try to reproduce and fix it and/or talk to Chris. > > > > > > > > Am Mi., 12. Juni 2019 um 18:27 Uhr schrieb Balay, Satish via petsc-dev > > > > <[email protected]>: > > > > On Wed, 12 Jun 2019, Lisandro Dalcin via petsc-dev wrote: > > > > > > > > > $ python test/runtests.py -v -i dmstag TestDMStag_2D_PXY > > > > > [0@kl-18232] Python 2.7 (/usr/local/opt/python@2/bin/python2.7) > > > > > [0@kl-18232] PETSc 3.11.2 development (conf: 'arch-darwin-c-debug') > > > > > [0@kl-18232] petsc4py 3.11.0 > > > > > (build/lib.macosx-10.14-x86_64-2.7/petsc4py) > > > > > testCoordinates (test_dmstag.TestDMStag_2D_PXY) ... ERROR > > > > > testDMDAInterface (test_dmstag.TestDMStag_2D_PXY) ... ERROR > > > > > testDof (test_dmstag.TestDMStag_2D_PXY) ... ok > > > > > testGetOther (test_dmstag.TestDMStag_2D_PXY) ... ok > > > > > testGetVec (test_dmstag.TestDMStag_2D_PXY) ... ERROR > > > > > testMigrateVec (test_dmstag.TestDMStag_2D_PXY) ... ERROR > > > > > > > > I get: > > > > > > > > ====================================================================== > > > > ERROR: testCoordinates (test_dmstag.TestDMStag_2D_PXY) > > > > ---------------------------------------------------------------------- > > > > Traceback (most recent call last): > > > > File "test/test_dmstag.py", line 39, in testCoordinates > > > > self.da.setUniformCoordinates(0,1,0,1,0,1) > > > > File "PETSc/DMStag.pyx", line 255, in > > > > petsc4py.PETSc.DMStag.setUniformCoordinates > > > > Error: error code 56 > > > > [0] DMStagSetUniformCoordinates() line 1077 in > > > > /home/balay/petsc.z/src/dm/impls/stag/stagutils.c > > > > [0] DMStagSetUniformCoordinatesExplicit() line 1118 in > > > > /home/balay/petsc.z/src/dm/impls/stag/stagutils.c > > > > [0] DMStagSetUniformCoordinatesExplicit_2d() line 135 in > > > > /home/balay/petsc.z/src/dm/impls/stag/stag2d.c > > > > [0] DMLocalToGlobalBegin() line 2614 in > > > > /home/balay/petsc.z/src/dm/interface/dm.c > > > > [0] DMLocalToGlobalBegin_Stag() line 230 in > > > > /home/balay/petsc.z/src/dm/impls/stag/stag.c > > > > [0] No support for this operation for this object type > > > > [0] Local to Global scattering with INSERT_VALUES is not supported for > > > > single rank in a direction with boundary conditions (e.g. periodic) > > > > inducing a non-injective local->global map. Either change the boundary > > > > conditions, use a stencil width of zero, or use more than one rank in > > > > the relevant direction (e.g. -stag_ranks_x 2) > > > > > > > > Satish > > > > > > > > > >
