search

pecos / tps

Torch Plasma Simulator
BSD 3-Clause "New" or "Revised" License
8 stars 2 forks source link

Potential problem in gpu path BC #199

Closed trevilo closed 1 year ago

trevilo commented 1 year ago

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).

trevilo commented 1 year ago

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

trevilo commented 1 year ago

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:

https://github.com/pecos/tps/blob/044f0f54c9970c8ddfa74cf8470a9cd2b34c3366/src/M2ulPhyS.cpp#L1006-L1010

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.