Lr multipatch refinement alternative #207

VikingScientist · 2017-10-09T10:01:23Z

Fixes LR multipatch refinement in two steps:

by using existing MLGN arrays to pass boundary functions to neighbouring patches (with the additional logic of finding boundary functions completely covered by requested refinement indices)
interior refinement is done by picking all overlapping functions of all boundary functions on both sides of the boundary. This is needed since LR meshes requires the mesh inside the patch to match near the boundary.

VikingScientist · 2017-10-09T10:03:51Z

Illustration of why we need interior mesh-matching in addition to just the one on the boundary. This is why we need the proposed step 2

VikingScientist · 2017-10-09T10:07:21Z

Note: The above mesh is quartic p=4 (since functions go across 6 mesh lines). The proposed solution errs on the side of caution and creates a matching mesh p+1=5 elements into the mesh. This is by picking all interior functions that overlap with the shared boundary functions.

akva2

picking on the cosmetics first. also debug output needs to go.

akva2 · 2017-10-09T10:46:14Z

src/ASM/LR/ASMLRSpline.C

@@ -307,6 +307,26 @@ IntVec ASMunstruct::getFunctionsForElements (const IntVec& elements)
  return result;
 }

+IntVec ASMunstruct::getBoundaryNodesCovered (const IntVec& nodes) const


additional space between functions.

Enig, Den uskrevne IFEM-standarden sier 2 blanke linjer.

akva2 · 2017-10-09T10:46:23Z

src/ASM/LR/ASMLRSpline.C

+  geo->generateIDs();
+  IntVec oneBoundary, result;
+  int numbEdges = (getNoParamDim() == 2) ? 4 : 6;
+  for(int edge=1; edge<=numbEdges;  edge++)


space between for and (

Du har ikke korrigert dette (samt for alle ditto)?? Dessuten dobbel space før edge++

akva2 · 2017-10-09T10:46:31Z

src/ASM/LR/ASMLRSpline.C

+  for(int edge=1; edge<=numbEdges;  edge++)
+  {
+    getBoundaryNodes(edge, oneBoundary); // this returns a 1-indexed list
+    for(const int i : nodes)


akva2 · 2017-10-09T10:46:37Z

src/ASM/LR/ASMLRSpline.C

+  {
+    getBoundaryNodes(edge, oneBoundary); // this returns a 1-indexed list
+    for(const int i : nodes)
+      for(const int j : oneBoundary)


akva2 · 2017-10-09T10:46:43Z

src/ASM/LR/ASMLRSpline.C

+    getBoundaryNodes(edge, oneBoundary); // this returns a 1-indexed list
+    for(const int i : nodes)
+      for(const int j : oneBoundary)
+        if(geo->getBasisfunction(i)->contains(*geo->getBasisfunction(j-1)))


akva2 · 2017-10-09T10:48:29Z

src/ASM/LR/ASMLRSpline.C

+{
+  IntVec result;
+  geo->generateIDs();
+  for(const int i : nodes)


akva2 · 2017-10-09T10:48:35Z

src/ASM/LR/ASMLRSpline.C

+  for(const int i : nodes)
+  {
+    LR::Basisfunction *b = geo->getBasisfunction(i);
+    for(auto el : b->support()) // for all elements where *b has support


akva2 · 2017-10-09T10:48:42Z

src/ASM/LR/ASMLRSpline.C

+  {
+    LR::Basisfunction *b = geo->getBasisfunction(i);
+    for(auto el : b->support()) // for all elements where *b has support
+      for(auto basis : el->support()) // for all functions on this element


akva2 · 2017-10-09T10:48:53Z

src/SIM/SIMinput.C

+  {
+    pch = dynamic_cast<ASMunstruct*>(myModel[i]);
+    IntVec secondary = pch->getOverlappingNodes(conformingIndicies[i]);
+    for(int j : conformingIndicies[i]) // add refinement from neighbours


akva2 · 2017-10-09T10:48:59Z

src/SIM/SIMinput.C

      refineIndices[i].push_back(j);
-
+    for(int j : secondary)             // add depth refinement to make it conforming


akva2 · 2017-10-12T07:28:34Z

bruker jeg den reduce footprint så sier frac 'frac you'.

VikingScientist · 2017-10-12T09:04:30Z

This is the setup which requires the reduced footprint commit. The proposed setup managed to pass covered boundary functions (blue) to their respective connected patches (green), but the problem arised when interior mesh was to be refined. By taking all functions with overlapping support (yellow) one might actually hit another patch boundary and this is not detected and the gray function is never added.

The proposed solution in 7da3af4 tries to only pick (yellow) functions which support does not grow in the boundary tangent direction, but only in the boundary normal direction.

VikingScientist · 2017-10-12T13:46:25Z

Updated... fixed all boundary cases, and made the code all pretty

VikingScientist · 2017-10-13T10:39:14Z

jenkins build this with downstreams ifem-poisson=13 ifem-elasiticity=37 please

VikingScientist · 2017-10-13T11:20:12Z

jenkins build this with downstreams ifem-poisson=13 ifem-elasticity=37 please

VikingScientist · 2017-10-13T13:21:59Z

jenkins build this with downstreams ifem-poisson=13 ifem-elasticity=37 ifem-stokes=77 please

VikingScientist · 2017-10-13T13:55:52Z

jenkins build this with downstreams ifem-poisson=13 ifem-elasticity=37 ifem-stokes=77 please

VikingScientist · 2017-10-16T09:07:52Z

Is this OK? Can we put this on trunk now? I can't make Jenkins run the tests for some reason 😢

akva2 · 2017-10-16T09:09:37Z

jenkins build this with downstreams ifem-poisson=13 ifem-elasticity=37 ifem-stokes=77 please

VikingScientist · 2017-10-17T06:40:46Z

Squashed fixup commits. Ready for merge. Also Elasticity, Poisson and Stokes

kmokstad

Prøvde å gå gjennom, og fant en del ting å pirke på.
Alt dette henger mer eller mindre sammen så vidt jeg kan se, så jeg tror man med fordel kan squashe alt (eller det meste) til end commit. Da er det enklere å gjøre effektiv review,

kmokstad · 2017-10-18T05:02:30Z

src/ASM/ASMunstruct.h

+  //! \param   nodes List of (0-indexed) patch local node IDs
+  //! \returns       Node IDs (0-indexed) for boundary functions whos support is
+  //                 completely covered by the union of the support in the input array
+  virtual IntVec getBoundaryNodesCovered(const IntVec& nodes) const ;


fjern space før ; og det holder med en space etter \param og \return i doxyen, og siden nodes kun er input, bruk \param[in]. One spell error too (whos --> whose).

kmokstad · 2017-10-18T05:04:03Z

src/ASM/LR/ASMLRSpline.C

@@ -307,6 +307,26 @@ IntVec ASMunstruct::getFunctionsForElements (const IntVec& elements)
  return result;
 }

+IntVec ASMunstruct::getBoundaryNodesCovered (const IntVec& nodes) const


Enig, Den uskrevne IFEM-standarden sier 2 blanke linjer.

kmokstad · 2017-10-18T05:06:04Z

src/ASM/LR/ASMLRSpline.C

+  geo->generateIDs();
+  IntVec oneBoundary, result;
+  int numbEdges = (getNoParamDim() == 2) ? 4 : 6;
+  for(int edge=1; edge<=numbEdges;  edge++)


Du har ikke korrigert dette (samt for alle ditto)?? Dessuten dobbel space før edge++

kmokstad · 2017-10-18T05:14:02Z

src/ASM/LR/ASMLRSpline.C

+  std::sort(result.begin(), result.end());
+  auto last = std::unique(result.begin(), result.end());
+  result.erase(last, result.end());
+  return result;


Magefølelsen sier at du vel trenger å inkludere <algorithm>for at dette skal bygge over alt?

Men, et bedre alternative er kanskje å bruke std::set<int> result i stedet og så return IntVec(result.begin(),result.end());. Da vil sorteringen og uniqueifiseringen komme på kjøpet.

kmokstad · 2017-10-18T05:16:10Z

src/SIM/SIMinput.C

@@ -27,6 +27,8 @@
 #include <sstream>
 #include <numeric>

+using namespace std;
+


Nei, vi vil ikke ha denne her eller andre steder. Vi bruker std::-scoping der det trengs,

kmokstad · 2017-10-18T05:30:09Z

src/SIM/SIMinput.C

+      cout << j << endl;
+    cout << " Covered (boundary) indices:\n";
+    for(int j : bndry_nodes)
+      cout << j << endl;


Dette er vel mer debug-output som bør gjemmes unna i en #ifdef SP_DEBUG eller noe?
Og legg til std::cout hvis utskriften ønske beholdes.

kmokstad · 2017-10-18T05:39:07Z

src/SIM/SIMinput.C

+    std::sort(refineIndices[i].begin(), refineIndices[i].end());
+    auto last = std::unique(refineIndices[i].begin(), refineIndices[i].end());
+    refineIndices[i].erase(last, refineIndices[i].end());
+


Her også, vurder å bruke std::set<int>i stedet.

Innser nå at mye av det jeg kommenterte på i første commit er endret på i denne. Da burde heller de to committene vært squashet til en for å gjøre review-prosessen enklere og ryddigere

Hvis du klikker på "Files changed" istedenfor "commits" øverst, så får du diff'en til hele pull requesten. Essensielt det du spør om: slik ting hadde sett ut hvis alt hadde blitt most til en enkelt commit.

Jeg tror ikke review hadde blitt enklere med få commits, tvert imot da man mister commit messages og en del logikk til hvorfor ting blir som de blir. Jeg kan derimot se argumentet med at man ikke vil ha for mange intermediate states i historikken der programmet dumper ut unødvendig mye debug print (som her hos meg) eller værst av alt: ikke kompilerer (skal ikke være tilfellet her). Hvis det fortsatt er ønskelig, så moser jeg alt sammen til en stor commit.

For all del, alt trenger ikke å være en stor commit, men de committene som er der bør vær logisk inndelt så langt det går. Ihvertfall trengs ikke fjernings av debug print, dokumentasjons-endringer, etc. være separat. De kan med fordel inkluderes i de forrige der de hører best hjemme.

kmokstad · 2017-10-18T05:45:39Z

src/ASM/ASMunstruct.h

@@ -142,8 +142,9 @@ class ASMunstruct : public ASMbase

  //! \brief Return all functions whos support overlap with the input functions
  //! \param nodes List of (0-indexed) patch local node IDs (typically requested by adaptive refinement)
+  //! \param dir   In which parametric direction the basis functions are allowed to have bigger support (-1 for all)


\param[in] dir

kmokstad · 2017-10-18T05:48:09Z

src/SIM/SIMinput.C

    for (size_t j = 0; j < myModel.size(); j++)
    {
      std::sort(conformingIndicies[j].begin(), conformingIndicies[j].end());
      auto last = std::unique(conformingIndicies[j].begin(), conformingIndicies[j].end());
      conformingIndicies[j].erase(last, conformingIndicies[j].end());
-        for(int l : conformingIndicies[j])
-          cout << "#"<<j<<": "<<l << endl;


Og der forsvant debug-output'en gitt. dvs. min kommentering på det i forrige commit var bortkasta. Dette understrerker at det er viktig å squase slike endringer også før review.

kmokstad · 2017-10-18T05:49:55Z

src/SIM/SIMinput.C

  for (size_t i = 0; i < myModel.size(); i++)
  {
+    std::sort(conformingIndicies[i].begin(), conformingIndicies[i].end());
+    auto last = std::unique(conformingIndicies[i].begin(), conformingIndicies[i].end());
+    conformingIndicies[i].erase(last, conformingIndicies[i].end());


Again, consider using std::set<int>?

VikingScientist · 2017-10-18T07:22:02Z

All requested changes fixed

…o match interior mesh

…daries

…pegation

…to refine

VikingScientist · 2017-10-19T13:19:10Z

Is this ready for merging?

kmokstad · 2017-10-19T20:27:48Z

Well, I looked at it again tonight as I cloned your branch anyway to check it on the Hemisphere problem. And I found several smaller/cosmetic issues I would like to correct. But to save you the burden for more "kverulering" from me, I did a rebase myself. See the branch kmokstad:lr-multipatch-alternative. You'll find most of your commits there (except two) such that the history is intact, but without some cosmetic backs-and-forth. The main difference is more use of std::set instead of vector, but the results should be unchanged. Check if you agree, then you can merge that one.

kmokstad · 2017-11-07T16:22:26Z

#215 have now been merged instead, so this one is therefore obsolete.

VikingScientist added the work in progress label Oct 9, 2017

VikingScientist requested a review from akva2 October 9, 2017 10:01

VikingScientist mentioned this pull request Oct 9, 2017

Easy access to neighboring node not on the boundary for LR-meshes #203

Open

akva2 requested changes Oct 9, 2017

View reviewed changes

akva2 mentioned this pull request Oct 10, 2017

add support for multipatch solution and history transfer kmokstad/IFEM-OpenFrac-1#2

Closed

VikingScientist force-pushed the lr-multipatch-alternative branch 2 times, most recently from a8d705d to b8d9a62 Compare October 12, 2017 13:37

akva2 removed the work in progress label Oct 12, 2017

VikingScientist mentioned this pull request Oct 13, 2017

Updated regression test with increased (LR) multipatch refinement footprint OPM/IFEM-Poisson#13

Merged

VikingScientist force-pushed the lr-multipatch-alternative branch from da9db1c to fffbbfa Compare October 13, 2017 10:30

VikingScientist mentioned this pull request Oct 13, 2017

Updated regression test with increased (LR) multipatch refinement footprint OPM/IFEM-Elasticity#37

Merged

akva2 approved these changes Oct 13, 2017

View reviewed changes

VikingScientist force-pushed the lr-multipatch-alternative branch 3 times, most recently from fffbbfa to 7138a12 Compare October 13, 2017 12:57

VikingScientist force-pushed the lr-multipatch-alternative branch from 7138a12 to 225e9af Compare October 17, 2017 06:40

kmokstad requested changes Oct 18, 2017

View reviewed changes

VikingScientist force-pushed the lr-multipatch-alternative branch from 39d0e53 to f8a0d31 Compare October 18, 2017 07:20

VikingScientist and others added 9 commits October 18, 2017 11:01

Find covered boundary functions during refinement and pass to neighbours

5ad72b1

LR multipatch: added refinement propegation across patch boundaries t…

dcdd9a9

…o match interior mesh

add support for multipatch solution transfer

4849d13

LR multipatch adap: reduce footprint by only extending normal to boun…

b9622b6

…daries

Bugfix, comments and tiny optimization

5de5833

Documentation

6a7c3fa

LR multipatch: special case corners, lines and faces for interior pro…

20b2eaf

…pegation

LR multipatch: optimize mesh, by reducing number of functions picked …

640941c

…to refine

LR multipatch: changed refine indices from std vector to set

e573c49

VikingScientist force-pushed the lr-multipatch-alternative branch from f8a0d31 to e573c49 Compare October 18, 2017 09:03

kmokstad mentioned this pull request Oct 22, 2017

Lr multipatch alternative, rebased #215

Merged

kmokstad closed this Nov 7, 2017

		refineIndices[i].push_back(j);

		for(int j : secondary) // add depth refinement to make it conforming

Lr multipatch refinement alternative #207

Lr multipatch refinement alternative #207

Conversation

VikingScientist commented Oct 9, 2017

VikingScientist commented Oct 9, 2017

VikingScientist commented Oct 9, 2017

akva2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akva2 commented Oct 12, 2017

VikingScientist commented Oct 12, 2017

VikingScientist commented Oct 12, 2017

VikingScientist commented Oct 13, 2017

VikingScientist commented Oct 13, 2017

VikingScientist commented Oct 13, 2017

VikingScientist commented Oct 13, 2017

VikingScientist commented Oct 16, 2017

akva2 commented Oct 16, 2017

VikingScientist commented Oct 17, 2017

kmokstad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VikingScientist commented Oct 18, 2017

VikingScientist commented Oct 19, 2017

kmokstad commented Oct 19, 2017

kmokstad commented Nov 7, 2017