Skip to content

Commit 0561eae

Browse files
committed
petsc-custom: add internal-boundary ownership patch, mpi test, and docs
1 parent 220fd1c commit 0561eae

6 files changed

Lines changed: 392 additions & 8 deletions

CHANGES.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,16 @@
11
# CHANGES: Underworld3
22

3+
## 2026-03-12
4+
5+
- Fixed PETSc DMPlex internal-boundary MPI rank-dependence in custom PETSc patch workflow.
6+
- Added `plexfem-internal-boundary-ownership-fix.patch`:
7+
- ghost-facet ownership filtering in boundary integral / residual / Jacobian paths
8+
- part-consistent residual assembly (`support[key.part]`) with support-size guards
9+
- Added MPI regression test:
10+
- `tests/parallel/test_0765_internal_boundary_integral_mpi.py`
11+
- Validation benchmark:
12+
- `Ex_Stokes_Kramer_latest.py` (`case1`, natural BC) now shows stable velocity L2 across `np=1,2,4,8` (no `np=8` branch split).
13+
314
## 2025-12-21
415

516
- PETSc 3.24 compatibility verified (conda-forge petsc 3.24.2 works correctly)

petsc-custom/README.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,15 @@ cd petsc-custom
3939
./build-petsc.sh clean # Remove everything
4040
```
4141

42+
## PETSc Patch Selection
43+
44+
- `build-petsc.sh` applies PETSc boundary-assembly patches from `petsc-custom/patches`.
45+
- Preferred patch:
46+
- `plexfem-internal-boundary-ownership-fix.patch`
47+
- Legacy fallback (only if preferred patch is absent):
48+
- `plexfem-ghost-facet-fix.patch`
49+
- Do not apply both patches together.
50+
4251
## What Gets Installed
4352

4453
The build downloads and compiles:

petsc-custom/build-petsc.sh

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,21 @@ apply_patches() {
6161
echo "Applying UW3 patches to PETSc..."
6262
cd "$PETSC_DIR"
6363

64-
# Fix ghost facet double-counting in boundary residual/integral assembly.
65-
# Without this, internal boundary natural BCs and BdIntegral produce
66-
# incorrect results in parallel (shared facets integrated on multiple ranks).
67-
# Submitted upstream: [TODO: add PETSc MR link when available]
68-
local patch="${SCRIPT_DIR}/patches/plexfem-ghost-facet-fix.patch"
69-
if [ -f "$patch" ]; then
70-
if git apply --check "$patch" 2>/dev/null; then
71-
git apply "$patch"
64+
# Internal-boundary ownership + part-consistent assembly fix in plexfem.c.
65+
# Supersedes the older ghost-facet-only patch.
66+
local patch_new="${SCRIPT_DIR}/patches/plexfem-internal-boundary-ownership-fix.patch"
67+
local patch_old="${SCRIPT_DIR}/patches/plexfem-ghost-facet-fix.patch"
68+
69+
if [ -f "$patch_new" ]; then
70+
if git apply --check "$patch_new" 2>/dev/null; then
71+
git apply "$patch_new"
72+
echo " Applied: plexfem-internal-boundary-ownership-fix.patch"
73+
else
74+
echo " Skipped: plexfem-internal-boundary-ownership-fix.patch (already applied or conflict)"
75+
fi
76+
elif [ -f "$patch_old" ]; then
77+
if git apply --check "$patch_old" 2>/dev/null; then
78+
git apply "$patch_old"
7279
echo " Applied: plexfem-ghost-facet-fix.patch"
7380
else
7481
echo " Skipped: plexfem-ghost-facet-fix.patch (already applied or conflict)"
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
# PETSc Patch: Internal Boundary Ownership Consistency
2+
3+
## Patch file
4+
5+
- `plexfem-internal-boundary-ownership-fix.patch`
6+
7+
## Scope
8+
9+
File touched in PETSc:
10+
11+
- `src/dm/impls/plex/plexfem.c`
12+
13+
## Problem
14+
15+
Internal-boundary contributions in parallel were rank-dependent for natural BC / boundary integral workflows.
16+
Symptoms included:
17+
18+
- Different `v_l2`/`p_l2` across MPI sizes for the same problem setup.
19+
- Different boundary callback totals between ranks/partitions.
20+
21+
Root causes:
22+
23+
1. Boundary facet sets were not consistently filtered to owned facets in all boundary assembly paths.
24+
2. `DMPlexComputeBdResidualSingleByKey` always used `support[0]` during closure gather/scatter, which is not part-consistent when `key.part != 0`.
25+
3. Part filtering for boundary points needed to guard against insufficient support for `part > 0`.
26+
27+
## Fix Summary
28+
29+
The patch makes boundary assembly ownership-consistent and part-consistent:
30+
31+
1. Filter SF leaf (ghost) facets from `facetIS` using `ISDifference(...)` in:
32+
- `DMPlexComputeBdIntegral`
33+
- `DMPlexComputeBdResidual_Internal`
34+
- `DMPlexComputeBdJacobian_Internal`
35+
36+
2. In `DMPlexComputeBdResidualSingleByKey`:
37+
- For `key.part > 0`, filter points with `supportSize <= key.part`.
38+
- Use `support[key.part]` (not hardcoded `support[0]`) for:
39+
- local closure reads (`locX`, `locX_t`)
40+
- auxiliary closure lookup
41+
- local residual insertion (`DMPlexVecSetClosure`)
42+
- Add `PetscCheck(supportSize > key.part, ...)` guards before use.
43+
44+
## Why this resolves the bug
45+
46+
Each physical boundary facet is assembled once (owned rank only), and each part uses the correct adjacent cell side. This removes over/under-assembly and residual/Jacobian side mismatches that previously depended on partition topology.
47+
48+
## Validation
49+
50+
Validated with Underworld3 annulus internal-boundary cases:
51+
52+
- `Ex_Stokes_Kramer_latest.py` with natural BC, `np=1,2,4,8`:
53+
- stable velocity L2 around `2.32345982e-03`
54+
- no branch split at `np=8`
55+
56+
- Boundary callback totals (`bdres`/`bdjac`) match between `np=7` and `np=8` after patch.
57+
58+
- A UW MPI regression test was added:
59+
- `tests/parallel/test_0765_internal_boundary_integral_mpi.py`
60+
- checks annulus internal-boundary circumference in parallel against analytic value.
61+
62+
## Benchmark Script And Case
63+
64+
- Benchmark script (GitHub):
65+
- https://github.com/gthyagi/UW3_Annulus_Spherical_Benchmarks/blob/main/benchmarks/annulus/Ex_Stokes_Kramer_latest.py
66+
67+
- Recommended case for reproducing/validating this fix:
68+
- `-uw_case case1 -uw_bc_type natural`
69+
70+
- Example runs:
71+
72+
```bash
73+
# from UW3_Annulus_Spherical_Benchmarks/benchmarks/annulus
74+
PY=/path/to/uw/.pixi/envs/amr-dev/bin/python
75+
SCRIPT=Ex_Stokes_Kramer_latest.py
76+
77+
for n in 1 2 4 8; do
78+
mpirun -np "$n" "$PY" "$SCRIPT" -uw_case case1 -uw_bc_type natural
79+
done
80+
```
81+
82+
## Patch Application Matrix
83+
84+
Use `git apply --check` before applying:
85+
86+
```bash
87+
git apply --check /path/to/patch.patch
88+
```
89+
90+
### Case A: old patch is not applied
91+
92+
- Recommended and simplest path.
93+
- Apply only:
94+
- `plexfem-internal-boundary-ownership-fix.patch`
95+
96+
### Case B: old patch is already applied
97+
98+
- Do not stack both full patches.
99+
- `plexfem-internal-boundary-ownership-fix.patch` usually overlaps with the old patch and will fail `--check`.
100+
- Recommended options:
101+
1. Revert/restore `plexfem.c` to clean PETSc source, then apply only the new patch.
102+
2. Or create/use an incremental patch from old -> new (if you need sequential layering).
103+
104+
## Verification Commands
105+
106+
### 1) UW MPI regression test
107+
108+
```bash
109+
mpirun -np 4 python -m pytest --with-mpi tests/parallel/test_0765_internal_boundary_integral_mpi.py -q
110+
```
111+
112+
Expected:
113+
- `2 passed`
114+
115+
### 2) Benchmark validation
116+
117+
```bash
118+
# from UW3_Annulus_Spherical_Benchmarks/benchmarks/annulus
119+
for n in 1 2 4 8; do
120+
mpirun -np "$n" "$PY" "$SCRIPT" -uw_case case1 -uw_bc_type natural
121+
done
122+
```
123+
124+
Expected:
125+
- `Relative velocity L2 error` should be stable across `np=1,2,4,8`
126+
- Typical value around `2.32345982e-03` for this case
127+
128+
## Notes
129+
130+
- This patch supersedes the narrower `plexfem-ghost-facet-fix.patch` by also addressing part-consistent residual assembly and Jacobian ownership filtering.
131+
- Apply **only one** of these two patches.
132+
- Preferred: `plexfem-internal-boundary-ownership-fix.patch`
133+
- Legacy fallback: `plexfem-ghost-facet-fix.patch`
134+
- Do **not** apply both patches together (overlapping hunks / duplicate logic).
Lines changed: 162 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
diff --git a/src/dm/impls/plex/plexfem.c b/src/dm/impls/plex/plexfem.c
2+
index 0299f569070..1f07f49f60f 100644
3+
--- a/src/dm/impls/plex/plexfem.c
4+
+++ b/src/dm/impls/plex/plexfem.c
5+
@@ -2874,6 +2874,24 @@ PetscErrorCode DMPlexComputeBdIntegral(DM dm, Vec X, DMLabel label, PetscInt num
6+
PetscCall(DMPlexGetDepthLabel(dm, &depthLabel));
7+
PetscCall(DMGetDimension(dm, &dim));
8+
PetscCall(DMLabelGetStratumIS(depthLabel, dim - 1, &facetIS));
9+
+ /* Filter out ghost facets (SF leaves) so each facet contributes once */
10+
+ if (facetIS) {
11+
+ PetscSF sf;
12+
+ PetscInt nleaves;
13+
+ const PetscInt *leaves;
14+
+
15+
+ PetscCall(DMGetPointSF(dm, &sf));
16+
+ PetscCall(PetscSFGetGraph(sf, NULL, &nleaves, &leaves, NULL));
17+
+ if (nleaves > 0 && leaves) {
18+
+ IS leafIS, ownedFacetIS;
19+
+
20+
+ PetscCall(ISCreateGeneral(PETSC_COMM_SELF, nleaves, leaves, PETSC_USE_POINTER, &leafIS));
21+
+ PetscCall(ISDifference(facetIS, leafIS, &ownedFacetIS));
22+
+ PetscCall(ISDestroy(&leafIS));
23+
+ PetscCall(ISDestroy(&facetIS));
24+
+ facetIS = ownedFacetIS;
25+
+ }
26+
+ }
27+
PetscCall(DMGetLocalSection(dm, &section));
28+
PetscCall(PetscSectionGetNumFields(section, &Nf));
29+
/* Get local solution with boundary values */
30+
@@ -4961,6 +4979,28 @@ PetscErrorCode DMPlexComputeBdResidualSingleByKey(DM dm, PetscWeakForm wf, Petsc
31+
PetscCall(ISDestroy(&pointIS));
32+
pointIS = isectIS;
33+
}
34+
+ if (key.part > 0) {
35+
+ IS filteredIS = NULL;
36+
+ const PetscInt *allPoints;
37+
+ PetscInt *keptPoints = NULL;
38+
+ PetscInt nAllFaces, nKept = 0, fidx;
39+
+
40+
+ PetscCall(ISGetLocalSize(pointIS, &nAllFaces));
41+
+ PetscCall(ISGetIndices(pointIS, &allPoints));
42+
+ PetscCall(PetscMalloc1(nAllFaces, &keptPoints));
43+
+ for (fidx = 0; fidx < nAllFaces; ++fidx) {
44+
+ const PetscInt point = allPoints[fidx];
45+
+ PetscInt supportSize;
46+
+
47+
+ PetscCall(DMPlexGetSupportSize(dm, point, &supportSize));
48+
+ if (supportSize <= key.part) continue;
49+
+ keptPoints[nKept++] = point;
50+
+ }
51+
+ PetscCall(ISRestoreIndices(pointIS, &allPoints));
52+
+ PetscCall(ISDestroy(&pointIS));
53+
+ PetscCall(ISCreateGeneral(PETSC_COMM_SELF, nKept, keptPoints, PETSC_OWN_POINTER, &filteredIS));
54+
+ pointIS = filteredIS;
55+
+ }
56+
PetscCall(ISGetLocalSize(pointIS, &numFaces));
57+
PetscCall(ISGetIndices(pointIS, &points));
58+
PetscCall(PetscMalloc4(numFaces * totDim, &u, (locX_t ? (size_t)numFaces * totDim : 0), &u_t, numFaces * totDim, &elemVec, (locA ? (size_t)numFaces * totDimAux : 0), &a));
59+
@@ -4978,21 +5018,24 @@ PetscErrorCode DMPlexComputeBdResidualSingleByKey(DM dm, PetscWeakForm wf, Petsc
60+
for (face = 0; face < numFaces; ++face) {
61+
const PetscInt point = points[face], *support;
62+
PetscScalar *x = NULL;
63+
- PetscInt i;
64+
+ PetscInt i, supportSize, cell;
65+
66+
PetscCall(DMPlexGetSupport(dm, point, &support));
67+
- PetscCall(DMPlexVecGetClosure(plex, section, locX, support[0], NULL, &x));
68+
+ PetscCall(DMPlexGetSupportSize(dm, point, &supportSize));
69+
+ PetscCheck(supportSize > key.part, PetscObjectComm((PetscObject)dm), PETSC_ERR_ARG_OUTOFRANGE, "Boundary point %" PetscInt_FMT " has support size %" PetscInt_FMT " but requested part %" PetscInt_FMT, point, supportSize, key.part);
70+
+ cell = support[key.part];
71+
+ PetscCall(DMPlexVecGetClosure(plex, section, locX, cell, NULL, &x));
72+
for (i = 0; i < totDim; ++i) u[face * totDim + i] = x[i];
73+
- PetscCall(DMPlexVecRestoreClosure(plex, section, locX, support[0], NULL, &x));
74+
+ PetscCall(DMPlexVecRestoreClosure(plex, section, locX, cell, NULL, &x));
75+
if (locX_t) {
76+
- PetscCall(DMPlexVecGetClosure(plex, section, locX_t, support[0], NULL, &x));
77+
+ PetscCall(DMPlexVecGetClosure(plex, section, locX_t, cell, NULL, &x));
78+
for (i = 0; i < totDim; ++i) u_t[face * totDim + i] = x[i];
79+
- PetscCall(DMPlexVecRestoreClosure(plex, section, locX_t, support[0], NULL, &x));
80+
+ PetscCall(DMPlexVecRestoreClosure(plex, section, locX_t, cell, NULL, &x));
81+
}
82+
if (locA) {
83+
PetscInt subp;
84+
85+
- PetscCall(DMGetEnclosurePoint(plexA, dm, encAux, support[0], &subp));
86+
+ PetscCall(DMGetEnclosurePoint(plexA, dm, encAux, cell, &subp));
87+
PetscCall(DMPlexVecGetClosure(plexA, sectionAux, locA, subp, NULL, &x));
88+
for (i = 0; i < totDimAux; ++i) a[face * totDimAux + i] = x[i];
89+
PetscCall(DMPlexVecRestoreClosure(plexA, sectionAux, locA, subp, NULL, &x));
90+
@@ -5028,10 +5071,13 @@ PetscErrorCode DMPlexComputeBdResidualSingleByKey(DM dm, PetscWeakForm wf, Petsc
91+
}
92+
for (face = 0; face < numFaces; ++face) {
93+
const PetscInt point = points[face], *support;
94+
+ PetscInt supportSize;
95+
96+
if (mesh->printFEM > 1) PetscCall(DMPrintCellVector(point, name, totDim, &elemVec[face * totDim]));
97+
PetscCall(DMPlexGetSupport(plex, point, &support));
98+
- PetscCall(DMPlexVecSetClosure(plex, NULL, locF, support[0], &elemVec[face * totDim], ADD_ALL_VALUES));
99+
+ PetscCall(DMPlexGetSupportSize(dm, point, &supportSize));
100+
+ PetscCheck(supportSize > key.part, PetscObjectComm((PetscObject)dm), PETSC_ERR_ARG_OUTOFRANGE, "Boundary point %" PetscInt_FMT " has support size %" PetscInt_FMT " but requested part %" PetscInt_FMT, point, supportSize, key.part);
101+
+ PetscCall(DMPlexVecSetClosure(plex, NULL, locF, support[key.part], &elemVec[face * totDim], ADD_ALL_VALUES));
102+
}
103+
PetscCall(DMSNESRestoreFEGeom(coordField, pointIS, qGeom, PETSC_TRUE, &fgeom));
104+
PetscCall(PetscQuadratureDestroy(&qGeom));
105+
@@ -5113,6 +5159,24 @@ static PetscErrorCode DMPlexComputeBdResidual_Internal(DM dm, Vec locX, Vec locX
106+
PetscCall(DMPlexGetDepthLabel(dm, &depthLabel));
107+
PetscCall(DMGetDimension(dm, &dim));
108+
PetscCall(DMLabelGetStratumIS(depthLabel, dim - 1, &facetIS));
109+
+ /* Filter out ghost facets (SF leaves) so each facet contributes once */
110+
+ if (facetIS) {
111+
+ PetscSF sf;
112+
+ PetscInt nleaves;
113+
+ const PetscInt *leaves;
114+
+
115+
+ PetscCall(DMGetPointSF(dm, &sf));
116+
+ PetscCall(PetscSFGetGraph(sf, NULL, &nleaves, &leaves, NULL));
117+
+ if (nleaves > 0 && leaves) {
118+
+ IS leafIS, ownedFacetIS;
119+
+
120+
+ PetscCall(ISCreateGeneral(PETSC_COMM_SELF, nleaves, leaves, PETSC_USE_POINTER, &leafIS));
121+
+ PetscCall(ISDifference(facetIS, leafIS, &ownedFacetIS));
122+
+ PetscCall(ISDestroy(&leafIS));
123+
+ PetscCall(ISDestroy(&facetIS));
124+
+ facetIS = ownedFacetIS;
125+
+ }
126+
+ }
127+
PetscCall(PetscDSGetNumBoundary(prob, &numBd));
128+
for (bd = 0; bd < numBd; ++bd) {
129+
PetscWeakForm wf;
130+
@@ -6117,13 +6181,31 @@ static PetscErrorCode DMPlexComputeBdJacobian_Internal(DM dm, Vec locX, Vec locX
131+
PetscInt dim, numBd, bd;
132+
DMLabel depthLabel;
133+
DMField coordField = NULL;
134+
- IS facetIS;
135+
+ IS facetIS = NULL;
136+
137+
PetscFunctionBegin;
138+
PetscCall(DMGetDS(dm, &prob));
139+
PetscCall(DMPlexGetDepthLabel(dm, &depthLabel));
140+
PetscCall(DMGetDimension(dm, &dim));
141+
PetscCall(DMLabelGetStratumIS(depthLabel, dim - 1, &facetIS));
142+
+ /* Filter out ghost facets (SF leaves) so each facet contributes once */
143+
+ if (facetIS) {
144+
+ PetscSF sf;
145+
+ PetscInt nleaves;
146+
+ const PetscInt *leaves;
147+
+
148+
+ PetscCall(DMGetPointSF(dm, &sf));
149+
+ PetscCall(PetscSFGetGraph(sf, NULL, &nleaves, &leaves, NULL));
150+
+ if (nleaves > 0 && leaves) {
151+
+ IS leafIS, ownedFacetIS;
152+
+
153+
+ PetscCall(ISCreateGeneral(PETSC_COMM_SELF, nleaves, leaves, PETSC_USE_POINTER, &leafIS));
154+
+ PetscCall(ISDifference(facetIS, leafIS, &ownedFacetIS));
155+
+ PetscCall(ISDestroy(&leafIS));
156+
+ PetscCall(ISDestroy(&facetIS));
157+
+ facetIS = ownedFacetIS;
158+
+ }
159+
+ }
160+
PetscCall(PetscDSGetNumBoundary(prob, &numBd));
161+
PetscCall(DMGetCoordinateField(dm, &coordField));
162+
for (bd = 0; bd < numBd; ++bd) {

0 commit comments

Comments
 (0)