pecos/tps

Potential problem in gpu path BC

Closed this issue · 2 comments

In setting up data structures that support the boundary face integration for the gpu code path, we use FiniteElementSpace::GetNBE(). However, as the comments in mfem indicate, this is not necessarily the true number of boundary faces. Specifically, comments within class Mesh state:

/* Calling fes->GetMesh()->GetNBE() doesn't return the expected value in 3D because
periodic meshes in 3D have some of their faces marked as boundary for
visualization purpose in GLVis. */

See similar comments in class FiniteElementSpace as well.

To fix this, we need to swap GetNBE() for GetNFbyType(FaceType::Boundary).

This is an easy fix. I am holding off until changes on gpu_cleanup_integration_arrays are finished and merged.

The difference between GetNBE() and GetNFbyType(FaceType::Boundary) is real, but it introduces no fundamental issue (beyond increased code complexity), because tr = mesh->GetBdrFaceTransformations(f); returns NULL for the periodic faces, so the following logic is sufficient:

tps/src/M2ulPhyS.cpp

Lines 1006 to 1010 in 044f0f5

for (int f = 0; f < NumBCelems; f++) {
tr = mesh->GetBdrFaceTransformations(f);
if (tr != NULL) {
fe = fes->GetFE(tr->Elem1No);

The result is that for cases with periodic boundaries, some of the entries (i.e., those that correspond to the periodic face indices) in the boundary data struct are not used. This design is suboptimal but not incorrect, so I won't fix it for now.