Hi Matt! I tested your branch and it works as expected, i.e. no empty label is created on my ExodusII mesh file.
I also left a comment on your MR on gitlab. Unfortunately I was not able to get to this this week, so thanks a lot for working on this - much appreciated. David > On Jul 9, 2022, at 14:15, Matthew Knepley <[email protected]> wrote: > > Hi David, > > Can you take a look at this and tell me whether it will work for you, or we > should change it? > > https://gitlab.com/petsc/petsc/-/merge_requests/5415 > <https://gitlab.com/petsc/petsc/-/merge_requests/5415> > > Thanks, > > Matt > > On Sun, Jun 26, 2022 at 4:08 PM David Andrs <[email protected] > <mailto:[email protected]>> wrote: > Hi Matt! > > On Jun 26, 2022, at 10:39, Matthew Knepley <[email protected] > <mailto:[email protected]>> wrote: >> >> On Sun, Jun 26, 2022 at 8:31 AM David Andrs <[email protected] >> <mailto:[email protected]>> wrote: >> Hi Jed! >> >> I have been thinking about this and I think more correct would be not to >> create the label with an empty name. It is not all or nothing kind of a >> deal. Some side sets may have a name, some may not. >> >> What I sent is probably more convenient, since after loading the mesh there >> is either a label with a name or with an ID which can be easily grabbed and >> passed into `PetscDSAddBoundary`. However, I agree that assigning the name >> with an ID like this is specific to an application. >> >> I can send a patch that will prevent the creation of a label with an empty >> name if you think this is what should be happening instead. >> >> I think what Jed is suggesting is to use "Face Sets" if the name is empty. I >> currently prefer this solution. > > I am not sure I follow. The “Face Sets” label gets created correctly in both > cases as far as I can tell. > > If there is an unnamed side set in the Exodus file, then a new label gets > created with no name. This label then contains all the unnamed side sets. > That does not seem correct to me and that’s what I was suggesting to fix in > case y'all also think that is not right. > >> Would this screw up your current workflow? > > No, it has no effect on my workflow. Jed’s question made me realize that I > need to create the labels with IDs as their names every time. In ExodusII, > IDs are mandatory, names are not. We do not know upfront if the user would > use an ID or a name, so I need to have both if they are present in the mesh > file. But, that is as I said, specific to my application. > > — > David > >> >> I would also like to have a consistent policy for everything, but it is hard >> for me to see what this would be right now. >> >> Thanks, >> >> Matt >> >> — >> David >> >> >> > On Jun 25, 2022, at 22:54, Jed Brown <[email protected] >> > <mailto:[email protected]>> wrote: >> > >> > Do you think this is more correct than just using "Face Sets", which gives >> > you a numbered index? >> > >> > There is curently some inconsistency between file formats in whether >> > various sets show up as stand-alone labels or values under Face Sets. And >> > there's this lingering issue to have a way to name Face Sets (and Cell >> > Sets). >> > >> > https://gitlab.com/petsc/petsc/-/issues/689 >> > <https://gitlab.com/petsc/petsc/-/issues/689> >> > >> > The issue is that if we just make named labels, there's no good way to do >> > structured traversal (handle all Face Sets) and one also has to be careful >> > to avoid namespace collisions between application sets and sets created by >> > the mesh. >> > >> > David Andrs <[email protected] <mailto:[email protected]>> writes: >> > >> >> Hello! >> >> >> >> The current behavior in DMPlexCreateExodus is assuming that side sets are >> >> always named. However, that is not always true. If there are unnamed side >> >> sets in an ExodusII file, the created DM will contain a label with an >> >> empty name (“”) with as many stratas as there are side sets. See the >> >> sample output from DMView below: >> >> >> >> Labels: >> >> celltype: 3 strata with value/size (0 (138900), 4 (125514), 1 (265620)) >> >> depth: 3 strata with value/size (0 (138900), 1 (265620), 2 (125514)) >> >> Cell Sets: 4 strata with value/size (1 (57888), 2 (57888), 4 (1824), 5 >> >> (7914)) >> >> Face Sets: 6 strata with value/size (1000 (28944), 1001 (28944), 1003 >> >> (28944), 1005 (15144), 1100 (120), 1200 (120)) >> >> : 6 strata with value/size (1000 (28944), 1001 (28944), 1003 (28944), >> >> 1005 (15144), 1100 (120), 1200 (120)) >> >> >> >> The attached patch fixes the behavior and names the side sets according >> >> to their IDs if there is no name associated with it in the ExodusII file. >> >> See the modified behavior below: >> >> >> >> Labels: >> >> celltype: 3 strata with value/size (0 (138900), 4 (125514), 1 (265620)) >> >> depth: 3 strata with value/size (0 (138900), 1 (265620), 2 (125514)) >> >> Cell Sets: 4 strata with value/size (1 (57888), 2 (57888), 4 (1824), 5 >> >> (7914)) >> >> Face Sets: 6 strata with value/size (1000 (28944), 1001 (28944), 1003 >> >> (28944), 1005 (15144), 1100 (120), 1200 (120)) >> >> 1000: 1 strata with value/size (1000 (28944)) >> >> 1001: 1 strata with value/size (1001 (28944)) >> >> 1003: 1 strata with value/size (1003 (28944)) >> >> 1005: 1 strata with value/size (1005 (15144)) >> >> 1100: 1 strata with value/size (1100 (120)) >> >> 1200: 1 strata with value/size (1200 (120)) >> >> >> >> The patch is against 3.17.0, but is seems it should apply cleanly on >> >> 3.17.2. >> >> >> >> With regards, >> >> >> >> David Andrs >> >> >> >> -- >> 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 >> >> https://www.cse.buffalo.edu/~knepley/ <http://www.cse.buffalo.edu/~knepley/> > > > -- > 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 > > https://www.cse.buffalo.edu/~knepley/ <http://www.cse.buffalo.edu/~knepley/>
