Mesh files: one directory, atomic writes, and a quick tier that is quick (#563)

[lmoresi]

Fixes #563.

The race

Two processes building the same mesh in one working directory raced. The generated file is named from the mesh parameters, so identical geometry is exactly the colliding case: one process opened <name>.msh.h5 for reading while the other was still writing it, and PETSc raised error 76. A parameter sweep launched as concurrent single-rank jobs hits this, and so does any parallel test run.

The fix is atomicity. Both writes — gmsh's .msh and the .msh.h5 PETSc converts it to — now land under a process-unique name and are renamed into place, so a reader sees a complete file or no file. _scratch_name keeps the extension, because gmsh chooses its output format from it.

The rename makes an MPI barrier before the read both necessary and sufficient — the other ranks must not go looking before rank 0 has renamed — so that barrier is now explicit rather than implied by the write happening to be fast.

UW_MESH_CACHE_DIR replaces the .meshes string literal that was hardcoded at every site across five meshing modules, and tests/conftest.py gives each xdist worker its own directory.

Negative control, because this is the kind of claim that is easy to get wrong: with every worker forced to share one directory, the run is equally green. So the atomicity is what fixes the race; the per-worker directory only stops four workers redoing the same gmsh work.

Two generators wrote to the wrong place

QuarterAnnulus and SegmentofAnnulus created .meshes/ and then wrote their .msh into the working directory — the prefix was simply missing from the name. That is where stray .msh files in run directories come from, and it made those two generators maximally exposed to the race.

The quick tier was not quick

./uw test advertises "~2 minutes" and took 9:45. The cause is pytest mark semantics: marks merge, so a module-level pytestmark = pytest.mark.level_1 plus a per-test @pytest.mark.level_2 leaves that test carrying both, -m level_1 selects it, and the demotion the author wrote does nothing. Nine files rely on that demotion — 24 escalations in total — and the heaviest of their tests is a 96-second homotopy solve, 17% of the whole tier on its own.

A level now selects by excluding the levels above it (-m "level_1 and not level_2 and not level_3"). That needs no change to any test file; the alternative was adding @pytest.mark.level_1 to 187 of 213 tests.

The recursion tests assumed an empty stack

setrecursionlimit(50) conflates "this operation does not recurse without bound" with "the whole stack is under 50 frames". Under xdist the worker's own frames spend the budget before the test body starts. The limit is now measured from the current depth, so the tests assert what they mean. (test_0600 keeps its absolute 100 — it has headroom and passes; left alone deliberately.)

Measured

Level 1 on a 16-core box, threads pinned:

	time	result
before	9:45	xdist impossible: 6 failed, 3 errors
after, serial	7:26	all green
after, -n 4 --dist loadfile	2:17	all green (1175 passed)

Level 2 also runs green under -n 4 (693 passed, 4:35), which is the regression check on the meshing change — every one of those tests builds meshes.

Not in this PR

Turning xdist on by default in ./uw test and in CI. That is the follow-up now that the race is dead; on CI's 4-vCPU runner it should take the 44-minute suite to roughly 13-15 minutes. Worth doing as its own change so the mechanism and the policy are reviewable separately.

Underworld development team with AI support from Claude Code

[lmoresi]

Adversarial review

We went after the claims rather than the code. Findings, with the evidence that survives them.

1. "Atomicity fixes the race" is the load-bearing claim, and it is the one we controlled for. Asserting it from the green run alone would be worthless — the per-worker directory would equally explain it. Forcing every worker to share one directory (UW_MESH_CACHE_DIR=.meshes/shared) is still green, so the rename is doing the work. That control is the difference between a fix and a coincidence, and it also tells users something real: a sweep sharing one directory is now safe, not merely tolerated.

2. The barrier is a behaviour change and deserves its own line. Before, non-zero ranks read a file rank 0 wrote in place; if they arrived early they saw a partial file and usually got away with it. With the rename they would instead see NO file and fail hard. The barrier converts "usually gets away with it" into "correct", but reviewers should note the failure mode moved from silent-corruption to loud-error before the barrier was added, which is why it is not optional.

3. _scratch_name keeping the extension is not cosmetic. gmsh selects its output format from the suffix, so the obvious f"{path}.{pid}.tmp" would have silently written something that is not a gmsh mesh — and the failure would have appeared much later, in createFromFile. Called out because the obvious refactor is wrong.

4. What we did NOT verify. True multi-rank behaviour of the new barrier is exercised only by the existing MPI suites, not by a test written for it; and we have not proven the race is gone by reproducing it deterministically — we observed 6 failures + 3 errors before and 0 after, across four runs. A deterministic reproduction (two processes, same geometry, one sleeping mid-write) would be a stronger regression guard than what we have. We think that is worth adding and did not add it here.

5. The marker change alters what ./uw test RUNS, not just how fast. 69 tests leave the quick tier (1244 -> 1175 selected). That is the intent — they are the demoted ones — but it means the quick tier is genuinely a smaller promise than it was yesterday, and anyone reading a green ./uw test should know it now excludes a 96-second homotopy solve that it previously included by accident. The tests still run at levels 2 and 3.

6. Scope we deliberately left. test_0600's absolute setrecursionlimit(100) is the same latent defect and is untouched because it passes with headroom; it will bite at higher -n. The os.makedirs(mesh_file_dir(), ...) call remains repeated at each site — the directory identity is now single-sourced, which is the part that mattered, but the "ensure it exists" line is still copy-paste.

Underworld development team with AI support from Claude Code

[Copilot]

Pull request overview

This PR addresses mesh-file races when multiple processes (including pytest-xdist workers) generate identical meshes in the same working directory, and restores the “quick” test tier to be genuinely quick by fixing marker-selection semantics and stabilizing recursion-limit regression tests under xdist.

Changes:

• Introduces a shared mesh-file helper (UW_MESH_CACHE_DIR, atomic gmsh writes) and updates meshing modules to write into a single configurable directory.
• Makes PETSc .msh.h5 generation atomic and adds an explicit MPI barrier before collective reads.
• Updates test tooling: per-xdist-worker mesh directories, corrected pytest marker selection for levels, and recursion tests that measure recursion headroom from current stack depth.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/test_0650_recursion_prevention_regression.py	Makes recursion-limit assertions relative to current stack depth for xdist compatibility.
tests/pytest.ini	Documents correct marker selection semantics (exclude higher levels).
tests/conftest.py	Sets UW_MESH_CACHE_DIR per xdist worker to avoid redundant mesh generation and collisions.
src/underworld3/meshing/spherical.py	Routes mesh output to mesh_file_dir() and uses atomic gmsh writes.
src/underworld3/meshing/segmented.py	Routes mesh output to mesh_file_dir() and uses atomic gmsh writes.
src/underworld3/meshing/geographic.py	Routes mesh output to mesh_file_dir() and uses atomic gmsh writes.
src/underworld3/meshing/cartesian.py	Routes mesh output to mesh_file_dir() and uses atomic gmsh writes; updates docstrings.
src/underworld3/meshing/annulus.py	Fixes previously missing .meshes/ prefix for some generators; uses atomic gmsh writes.
src/underworld3/meshing/_mesh_files.py	Adds centralized mesh directory selection and atomic gmsh write helper.
src/underworld3/discretisation/discretisation_mesh.py	Makes .h5 write atomic and adds an explicit barrier before _from_plexh5() reads.
scripts/test_levels.sh	Fixes level selection by excluding higher-level markers so “level_1” stays fast.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.