Skip to content

A split fault level keeps its structural rows in a zone-keyed FAC patch (#921) - #933

Open
lmoresi wants to merge 12 commits into
bugfix/fac-cover-gate-structure-918from
bugfix/fac-zone-structural-921
Open

lmoresi wants to merge 12 commits into
bugfix/fac-cover-gate-structure-918from
bugfix/fac-zone-structural-921

Conversation

@lmoresi

@lmoresi lmoresi commented Nov 3, 2026

Copy link
Copy Markdown
Member

With fac_zone keyed, the finest level's FAC patch is the zone's blocks. A split fault's structural rows joined them only under UW_FAC_STRUCTURAL=1, which defaulted to 0, while the fac_zone docstrings said they were always added. On a placed split fault those rows are the band, the split duplicates and the fill around them. The coarse level cannot represent them, and whole-level SOR converges them slowly.

This PR is stacked on the #918 PR (bugfix/fac-cover-gate-structure-918), because both change the same call sites.

Change

Row 5 of the add-on matrix

UWTN 2026-022's problem A at h_b/S = 24: a split frictionless fault from (0.30, 0.45) to (0.70, 0.55), and a TI band (eta_1 = 0.01 on planes along it) placed as a ribbon 0.08 below it, not cut. The hierarchy is the welded base, then the split S level with the FAC LU patch. Serial; velocity iterations are summed over the second solve's 17 applications.

patch key blocks velocity its
fac_zone = TI band, development 4 TI 820
fac_zone = TI band, this branch 4 TI + 1 structural 94
fac_zone = [TI band, fault band], development 8 344
no zone: the structural patch, gate open 1 66
lean chain of split levels, for reference none 230

The development rows ran on the laptop (macOS; development 25d6281 with #906, inert here), where the 94 was measured under UW_FAC_STRUCTURAL=1. On kaiju (Linux) this branch takes 94 serially as well. Keying the fault's band mask is not enough (344): the mask holds the band cells but not the fill between the band and the background, which the structural rows include. The peak slip is 0.13293 in every arm.

At np 4 on kaiju, development took 1179 iterations with the TI blocks alone and 439 with the fault's band mask keyed too; the structural patch alone, gate open, took 131. In parallel the union this branch makes the default does not converge. At np 4 the outer solve does not finish its first iteration (#925). The defect is in how the blocks are cut rank by rank, and development shows it under UW_FAC_STRUCTURAL=1 too. The PR stacked on this one fixes it: 110 and 117 iterations at np 2 and 4, against 94 serially. This branch should not reach development without it.

Tests

tests/parallel/test_1135_fac_zone_split_structural_parallel.py (level 2, tier B; np 1 in CI's serial phase, np 2 in the MPI phase) keys a zone strip near the left wall, away from the fault, on the linear fault fixture at 80 rungs. It compares the zone-keyed patch with the structural patch the same level gets without a zone:

On kaiju (Linux):

scripts/test.sh gains a line in serial batch 4.

Closes #921. Left open: #925 (fixed by the next PR in this stack), #895 and #897; verdicts in the review comment.

Underworld development team with AI support from Claude Code

…eyed

UWTN 2026-022's linear fault fixture at 80 rungs, with a fac_zone strip near
the left wall, away from the fault. On a split level every structural row must
be in the zone-keyed patch; on the #918 base none of the 4940 is. On the same
level placed but not split, the patch stays the zone alone (#670's default),
and the structural patch is not empty, so a union would show.

Underworld development team with AI support from Claude Code
…patch

With fac_zone keyed, the structural rows joined the zone's blocks only under
UW_FAC_STRUCTURAL=1, while the fac_zone docstrings said they were always
added. On a split fault those rows (the split duplicates and the fill around
them) are rows the coarse level cannot represent, and whole-level SOR does
not converge them: the add-on matrix's row 5, a TI band keyed beside a split
fault, took 820 velocity iterations with the TI blocks alone and 94 with the
structural block.

A finest level that carries a split fault (fault_mg.is_split_level, the same
on every rank) now always adds its structural rows as one more block; a level
without a split keeps #670's default, the zone's blocks alone. The
UW_FAC_STRUCTURAL campaign knob is gone. The docstrings and the subsystem doc
say what ships; test_1135 runs in serial batch 4 and in the parallel phase.

Underworld development team with AI support from Claude Code
…one-structural-921

Both branches change the finest level's FAC call sites. On the parallel zone
path the structural split runs on a split level or when no zone is keyed,
and it is ungated exactly when the level is split (cover_max=None), so a
split level's structural rows join a keyed zone whatever their size.
test_1134 (both files) and test_1135 are in serial batch 4.

Underworld development team with AI support from Claude Code
…one-structural-921

The split level's structural rows join a keyed zone under that level's 98 %
gate, in serial and parallel (CustomMGHierarchy._fac_cover_max), so a cut
uniform pair with a zone keyed adds no whole-level block. The comments and
the doc scope "whole-level SOR does not converge them" to the placed band.

From the second review: test_1135's split case also asserts that the zone
strip's own rows are in the patch, so a dropped zone cannot pass it.

Underworld development team with AI support from Claude Code
…one-structural-921

The third review's #918 fixes: the welded add-on route and the pinned split
threshold, the convergence wording, citations and tiers.

Underworld development team with AI support from Claude Code
…site

From the third review:
- With UW_FAC_STRUCTURAL gone, no caller passes cover_max=None, so the
  ungated branch in both split functions is removed.
- The serial zone path takes its threshold from _fac_cover_max(l), as every
  other call site does.
- On a zone-keyed level, fac_gate's verdict is the structural block's alone;
  the zone's blocks are installed either way (the doc says so).
- "Whole-level SOR does not converge" the structural rows is "converges them
  slowly": the 820 velocity iterations are summed over 17 applications.
- The design record notes that UW_FAC_STRUCTURAL and UW_FAC_COVER were
  removed; test_1135 carries its tier on each test.

Underworld development team with AI support from Claude Code
…maining

Underworld development team with AI support from Claude Code
…) into bugfix/fac-zone-structural-921

Underworld development team with AI support from Claude Code
Underworld development team with AI support from Claude Code
Underworld development team with AI support from Claude Code
820, 94 and 344 were measured serially (laptop; kaiju gives the same 94 on
this branch). The doc now says so; the np 4 counts on development were
1179, 131 and 439.

Underworld development team with AI support from Claude Code
@lmoresi

lmoresi commented Nov 3, 2026

Copy link
Copy Markdown
Member Author

Pre-PR adversarial review

Reviewed with #918, on which it is stacked: round 2 on 10c375e (52c22ae merged with #918's c8ecbf9), round 3 on 726bff2, then a final check of d02c369. The reviewers' probes were serial. The parallel defect below was found on kaiju after the reviews.

Round 2 (10c375e)

  • LOW: test_1135's split case could pass with the zone dropped, because the structural patch alone also holds every structural row. Fixed in 726bff2: the test also asserts that the strip's own rows are in the patch.
  • LOW: the 2026-09 placement design record listed UW_FAC_STRUCTURAL as a remaining knob. Fixed in d02c369 and 945504b.
  • LOW, outside the diff: "a split fault needs no patch" (five comments) and rule 0 of the 2026-08 patch design record ("The zone-block builder unions the uncovered structural rows in as an additional block automatically") read against the default as it stood. Left: the comments mean no zone key, and on a split level rule 0 holds again.
  • LOW: campaign drivers set UW_FAC_STRUCTURAL and UW_FAC_COVER, which are now ignored. The PR body says so.
  • Answered: on a split finest level the structural rows are always in the patch, and on an unsplit one never. No other route builds patch rows; every split route resets a mesh-owned zone to None.

Round 3 (726bff2)

  • LOW: with no caller passing cover_max=None, the ungated branches of both split functions were dead. Removed in d02c369.
  • LOW: one rule written two ways. The serial zone path hard-coded _FAC_COVER_MAX_SPLIT. Fixed in d02c369: every call site takes _fac_cover_max(l).
  • LOW: on a zone-keyed level, "declined" in fac_gate and the verbose line is the structural block's verdict only. The doc says so (d02c369).
  • LOW: "whole-level SOR does not converge" the structural rows overstated the record: 820 is a sum over 17 applications. Fixed in d02c369: "converges slowly".
  • LOW: a comment cited "24 times finer than the coarse level here" with nothing in the code to show it. Rewritten in d02c369.
  • LOW: the tier sat on the module. Fixed in d02c369: on each test.
  • On a cut uniform pair with a zone keyed, the zone's blocks are installed (398 rows), the structural block declines at 3896 of 3896 rows, and the solve takes 4 iterations: the same installed patch as on development.

Final check (d02c369)

  • Nothing in the repo. All three call sites pass _fac_cover_max(l), and git grep finds no cover_max=None and no reader of either removed knob.
  • LOW: the design record still listed UW_FAC_STRUCTURAL among the knobs that remain, in the sentence that says it was removed. Fixed in 945504b.

Found on kaiju: the union does not converge in parallel (#925)

  • Row 5's TI case at np 4 went silent on 945504b. The same case on development with UW_FAC_STRUCTURAL=1 and the gate open did too. Filed as FAC zone plus a split fault's structural rows: the solve hangs at np 4 (converges in serial) #925.
  • A traced run (2328b4c, np 4) found no deadlock. The outer solve did not finish its first iteration in 20 minutes; 31 of 33 velocity solves ran to the 200 cap.
  • The cause is how the finest level's blocks are cut rank by rank. The fix is the PR stacked on this one. Serially this branch takes 94 iterations on kaiju, as on the laptop.

Attacks that failed

Verified on kaiju (Linux)

3324ee6 merges the #918 branch (development through #927); c414465 says the doc's row 5 counts are serial.

Merge conditions

Issues in this territory

Issue Verdict
#921 Fixed by this branch: test_1135, and row 5 serially, 820 → 94 velocity iterations.
#925 Still live on this branch: at np 4 row 5's TI case does not finish an outer iteration. Fixed by the PR stacked on this one.
#895 Still live: the structural block beside a zone is one unbounded LU.
#897 Still live: the solver method set_custom_fmg takes no fac_zone.
#918 Fixed by the base PR of this stack.
#896 Already fixed (fixed-in-development); untouched.
#670, #671 Placement issues; untouched.

Underworld development team with AI support from Claude Code

#930)

custom_mg: the split function's docstring keeps #921's text (no cover_max=None)
with #918's 2-D scope.

Underworld development team with AI support from Claude Code

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant