GEOS-DEV / GEOS

GEOS Simulation Framework
GNU Lesser General Public License v2.1
203 stars 80 forks source link

Disable generation of globalIDs in GEOS for fracture meshes #3200

Closed ryar9534 closed 1 day ago

ryar9534 commented 2 days ago

We have seen that using GEOS to generate global IDs in examples where the mesh is fractured/split (and thus has collocated nodes) is unreliable as vtk will give the same global id to collocated nodes. Here I think I have simply returned an error in these cases so it is clear what is happening.

We could also just leave the code as is, as the code will likely catch the duplicated globalIDs, but if we like this we can merge it. Note also that even if GEOS uses vtk to assign global IDs, we still use the collocated nodes field from the vtk mesh for the fault to define the connectivity of the fracture to the matrix. At that point we could be using different numbering schemes which could lead to some very wacky stuff.

rrsettgast commented 2 days ago

@ryar9534 is vtk or GEOS generating the globalIDs?

ryar9534 commented 2 days ago

@ryar9534 is vtk or GEOS generating the globalIDs?

I believe its vtk under the hood, I think its happening here https://github.com/GEOS-DEV/GEOS/blob/55961896c6fd565a7452444bacce553cb5a8a0a0/src/coreComponents/mesh/generators/VTKUtilities.cpp#L202

rrsettgast commented 2 days ago

@ryar9534 is vtk or GEOS generating the globalIDs?

I believe its vtk under the hood, I think its happening here

https://github.com/GEOS-DEV/GEOS/blob/55961896c6fd565a7452444bacce553cb5a8a0a0/src/coreComponents/mesh/generators/VTKUtilities.cpp#L202

So VTK is checking coordinates and assuming colocated nodes are the same? Can't we just offset the nodes normal to the fault/fracture?

ryar9534 commented 2 days ago

@ryar9534 is vtk or GEOS generating the globalIDs?

I believe its vtk under the hood, I think its happening here https://github.com/GEOS-DEV/GEOS/blob/55961896c6fd565a7452444bacce553cb5a8a0a0/src/coreComponents/mesh/generators/VTKUtilities.cpp#L202

So VTK is checking coordinates and assuming colocated nodes are the same? Can't we just offset the nodes normal to the fault/fracture?

I suppose you could, I think thats just not the workflow @TotoGaz has been developing

TotoGaz commented 2 days ago

So VTK is checking coordinates and assuming colocated nodes are the same?

yes https://vtk.org/doc/nightly/html/classvtkGenerateGlobalIds.html

Can't we just offset the nodes normal to the fault/fracture? I suppose you could, I think thats just not the workflow @TotoGaz has been developing

Fractures use the global node ids to refer to matrix nodes. Having another process renumber the nodes is bound to fail.

codecov[bot] commented 2 days ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 55.74%. Comparing base (c74702a) to head (6f57cd8). Report is 2 commits behind head on develop.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #3200 +/- ## ======================================== Coverage 55.74% 55.74% ======================================== Files 1038 1038 Lines 88470 88471 +1 ======================================== + Hits 49320 49321 +1 Misses 39150 39150 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rrsettgast commented 2 days ago

So VTK is checking coordinates and assuming colocated nodes are the same?

yes https://vtk.org/doc/nightly/html/classvtkGenerateGlobalIds.html

Can't we just offset the nodes normal to the fault/fracture? I suppose you could, I think thats just not the workflow @TotoGaz has been developing

Fractures use the global node ids to refer to matrix nodes. Having another process renumber the nodes is bound to fail.

Does a call to generateGlobalIDs( vtkSmartPointer< vtkDataSet > mesh ) properly assign globalID to nodes? When does it fail? Why does it fail?

ryar9534 commented 2 days ago

So VTK is checking coordinates and assuming colocated nodes are the same?

yes https://vtk.org/doc/nightly/html/classvtkGenerateGlobalIds.html

Can't we just offset the nodes normal to the fault/fracture? I suppose you could, I think thats just not the workflow @TotoGaz has been developing

Fractures use the global node ids to refer to matrix nodes. Having another process renumber the nodes is bound to fail.

Does a call to generateGlobalIDs( vtkSmartPointer< vtkDataSet > mesh ) properly assign globalID to nodes? When does it fail? Why does it fail?

I'll try and answer this to take the load off @TotoGaz. For 'split' meshes with faults/fractures, we have been assuming the 'matrix' mesh (so the mesh of the 3D solid), has multiple nodes which are co-located on the fault. In this case assigning node global ids using vtk within geos will not work, as it will assign the same global ID to collocated nodes. Even if we assume that this was not the case, i.e. for whatever reason the call to vtk within geos properly assigned unique global IDs to collocated nodes, there could still be an issue. In particular, when we import the fracture we still make use of a 'collocated_nodes' field defined on the nodes of the fracture mesh. This field gives the global IDs of the 'matrix' nodes which are collocated with that node in the fracture. Even if the call to vtk gave valid (unique) global IDs to the matrix nodes, this ordering may not be the same as how 'collocated_nodes' was defined. I believe this is what Thomas was referring to in his comment. So it seems to me like the safest option (at least for now) is to throw an error if we attempt to create new globalIDs for a vtk mesh which includes a fault (indicated by the fractures variable being non-empty in the above changes). Hopefully that is (a) correct according to @TotoGaz and (b) answers your question @rrsettgast. Im happy to chat more whenever though!