Run the developer test loop across worker processes (CI held serial pending #567)

.github/workflows/build_uw3_and_test.yaml

@@ -28,6 +28,17 @@ env:
   # the UCX transports and retire the whole flake class.
   UCX_TLS: tcp,sm,self
 
+  # WORKERS is deliberately NOT set: the batches run in-process until #567 is
+  # fixed. Distributing them works and was measured at 4 workers (49m43s ->
+  # 30m53s) but changes which files share a process, which makes three
+  # point-locator tests answer in the wrong cell. Setting `WORKERS: 4` here is
+  # the whole change once #567 lands — and the only change, because
+  # scripts/test.sh already honours it.
+  #
+  # Note if you do set it: detect nothing. The first attempt derived the count
+  # inside the job, got 1, and ran xdist with a single worker — a spawn and a
+  # fresh underworld3 import per batch, parallelising nothing, for 52m37s.
+
 jobs:
   test:
     runs-on: ubuntu-latest

scripts/test.sh

@@ -48,7 +48,35 @@ fi
 export UW_NO_USAGE_METRICS=0
 # A hard crash must print a Python stack, not just "Segmentation fault".
 export PYTHONFAULTHANDLER=1
-PYTEST="pytest --config-file=tests/pytest.ini"
+
+# Each worker is a full PETSc/BLAS process. Without pinning the thread pools,
+# N workers each start their own and oversubscribe the runner badly enough to
+# run slower than serial.
+export OMP_NUM_THREADS=1
+export OPENBLAS_NUM_THREADS=1
+export MKL_NUM_THREADS=1
+
+# CI runs the batches in-process, deliberately. Distributing them across
+# workers WORKS and is measured — 49m43s to 30m53s at 4 workers on the
+# runner — but it also changes which files share a process, and that exposes
+# a real defect: three point-locator tests then answer in a cell that does
+# not contain the query point, for every point (issue #567). We are not
+# marking those xfail to buy the speedup.
+#
+# So CI stays serial until #567 is fixed. The developer loop does use workers
+# (scripts/test_levels.sh, `./uw test`: 9:45 to 1:22), because its grouping
+# does not hit the defect and the fast feedback is what stops people skipping
+# tests. Turning CI on afterwards is this block plus WORKERS in the workflow.
+#
+# WORKERS is honoured if set, so the parallel run stays one env var away for
+# anyone bisecting #567 in CI.
+if [ -n "$WORKERS" ] && [ "$WORKERS" -gt 1 ]; then
+    echo "Serial batches: $WORKERS worker process(es) (WORKERS set; see #567)"
+    PYTEST="pytest --config-file=tests/pytest.ini --dist loadfile -n $WORKERS"
+else
+    echo "Serial batches: in-process (workers held back pending #567)"
+    PYTEST="pytest --config-file=tests/pytest.ini"
+fi
 
 # Run serial tests (unless --parallel-only specified)
 if [ $PARALLEL_ONLY -eq 0 ]; then

scripts/test_levels.sh

@@ -21,21 +21,24 @@
 #   --parallel          Run parallel (MPI) tests with 2 ranks
 #   --parallel-ranks N  Run parallel (MPI) tests with N ranks
 #   --full-parallel     Run parallel tests with both 2 and 4 ranks
-#   --isolation         Enable per-file process isolation (prevents test pollution)
+#   --isolation         Run on ONE worker, still per-file (for pinning down
+#                       a pollution failure; slower than the default)
+#   --workers N, -j N   Worker processes (default: min(cores, 8))
 #   --verbose           Show verbose test output
 #   --help              Show this help message
 #
 # Defaults:
-#   Tests run WITHOUT process isolation and WITHOUT parallel (MPI) tests.
-#   This is the fastest mode for quick feedback during development.
+#   Tests run across min(cores, 8) worker processes, one file at a time per
+#   worker, and WITHOUT parallel (MPI) tests. Level 1 measured on 16 cores:
+#   9:45 serial, 1:32 at 8 workers.
 #
 # Process Isolation (--isolation):
-#   Runs each test file in a fresh subprocess via pytest-xdist (--dist loadfile -n 1).
-#   Prevents test pollution from global state (Model, units, PETSc contexts).
-#   Slower but more reliable for CI and comprehensive testing.
+#   Drops to ONE worker, still one file at a time. Every run is already
+#   per-file isolated; this removes the concurrency as well, which is what you
+#   want when a test passes alone and fails in a full run.
 #
 # Examples:
-#   ./test_levels.sh 1                       # Quick tests, fast mode
+#   ./test_levels.sh 1                       # Quick tests, all workers
 #   ./test_levels.sh --isolation 1,2         # Levels 1+2 with process isolation
 #   ./test_levels.sh --parallel 2            # Level 2 with MPI tests (2 ranks)
 #   ./test_levels.sh --parallel-ranks 4 2    # Level 2 with MPI tests (4 ranks)
@@ -71,6 +74,10 @@ while [[ $# -gt 0 ]]; do
             RUN_ISOLATION=1
             shift
             ;;
+        --workers|-j)
+            WORKERS="$2"
+            shift 2
+            ;;
         --verbose|-v)
             VERBOSE="-v"
             shift
@@ -106,19 +113,42 @@ export UW_NO_USAGE_METRICS=0
 # Disable telemetry during tests to prevent race conditions with kdtree
 export UW_ENABLE_TELEMETRY=0
 
-# Build pytest command with optional isolation
-# --dist loadfile: each test file runs in its own subprocess
-# -n 1: single worker (sequential but isolated per file)
+# Tests run across several worker processes by default.
+#
+# --dist loadfile keeps every test in a file on ONE worker, which is the
+# granularity the suite is safe at: tests within a file are written to follow
+# each other, while global state (Model, units, PETSc contexts) does not
+# survive between workers.
+#
+# Each worker is a full PETSc/BLAS process, so the thread pools have to be
+# pinned; without this, N workers each start their own and oversubscribe the
+# machine badly enough to run SLOWER than serial.
+export OMP_NUM_THREADS=1
+export OPENBLAS_NUM_THREADS=1
+export MKL_NUM_THREADS=1
+
+# Worker count. Measured on a 16-core box, level 1: serial 9:45, -n 4 2:17,
+# -n 8 1:32, -n 16 1:31 — so throughput saturates around 8 and the last
+# doubling buys nothing. It also costs something: at one worker per core the
+# file-to-worker grouping changes and three point-locator tests fail on state
+# left by whatever shared their process. Default to 8, capped by the machine.
+if [ -z "$WORKERS" ]; then
+    _cores=$( (command -v nproc >/dev/null && nproc) \
+              || sysctl -n hw.ncpu 2>/dev/null || echo 4 )
+    WORKERS=$(( _cores < 8 ? _cores : 8 ))
+fi
+
 # --timeout=120: prevent tests from hanging indefinitely (2 min per test max)
-# This prevents test pollution from global state (Model, units, PETSc)
 # Show test configuration
 echo "Configuration:"
 if [ $RUN_ISOLATION -eq 1 ]; then
+    # One worker, still per-file: sequential AND isolated, for pinning down a
+    # pollution failure rather than for speed.
     ISOLATION_OPTS="--dist loadfile -n 1"
-    echo "  🔒 Process isolation: ON"
+    echo "  🔒 Process isolation: ON (1 worker, one file at a time)"
 else
-    ISOLATION_OPTS=""
-    echo "  ⚡ Process isolation: OFF (fast mode)"
+    ISOLATION_OPTS="--dist loadfile -n $WORKERS"
+    echo "  ⚡ Workers: $WORKERS (one file at a time per worker)"
 fi
 if [ $RUN_PARALLEL -eq 1 ]; then
     echo "  🔀 Parallel (MPI): ON ($PARALLEL_RANKS ranks)"
@@ -146,26 +176,34 @@ run_tests() {
 }
 
 # Functions for each test level using pytest markers
+#
+# A level selection EXCLUDES the levels above it, and has to: pytest MERGES
+# marks rather than overriding them, so a file whose module declares
+# `pytestmark = pytest.mark.level_1` and whose heavy test then carries
+# `@pytest.mark.level_2` leaves that test marked BOTH. A plain `-m level_1`
+# selects it, and the demotion the author wrote does nothing. Nine files rely
+# on that demotion; the heaviest of their tests is a 96-second homotopy solve
+# that was running in the "quick" tier because of it.
 run_level_1() {
     echo "⚡ Running LEVEL 1: Quick Tests (Core Functionality)"
-    echo "Using pytest marker: -m level_1"
+    echo "Using pytest marker: -m 'level_1 and not level_2 and not level_3'"
     echo "Expected runtime: ~2 minutes"
     echo ""
 
-    # Run all tests marked with level_1
+    # Tests marked level_1 and NOT demoted to a higher level (see above)
     run_tests "Level 1 tests (quick core functionality)" \
-        tests/ -m level_1
+        tests/ -m "level_1 and not level_2 and not level_3"
 }
 
 run_level_2() {
     echo "🔧 Running LEVEL 2: Intermediate Tests"
-    echo "Using pytest marker: -m level_2"
+    echo "Using pytest marker: -m 'level_2 and not level_3'"
     echo "Expected runtime: ~5 minutes"
     echo ""
 
-    # Run all tests marked with level_2
+    # Tests marked level_2 and NOT demoted to level_3 (see above)
     run_tests "Level 2 tests (units, integration, projections)" \
-        tests/ -m level_2
+        tests/ -m "level_2 and not level_3"
 
     # Parallel tests for global statistics (requires MPI)
     if [ $RUN_PARALLEL -eq 1 ]; then

src/underworld3/discretisation/discretisation_mesh.py

@@ -143,9 +143,20 @@ def _from_gmsh(filename, comm=None, markVertices=False, useRegions=True, useMult
             plex_0.setName("uw_mesh")
             plex_0.markBoundaryFaces("All_Boundaries", 1001)
 
-            viewer = PETSc.ViewerHDF5().create(filename + ".h5", "w", comm=PETSc.COMM_SELF)
+            # Write aside and rename, so ``filename + ".h5"`` never names a
+            # half-written file. The name comes from the mesh PARAMETERS, so a
+            # second process building the same geometry in this directory picks
+            # the same one and would otherwise read what this one is still
+            # writing (issue #563).
+            # Imported here, not at module scope: the meshing package imports
+            # this module, so a top-level import would close a cycle.
+            from underworld3.meshing._mesh_files import _scratch_name
+
+            scratch = _scratch_name(filename + ".h5")
+            viewer = PETSc.ViewerHDF5().create(str(scratch), "w", comm=PETSc.COMM_SELF)
             viewer(plex_0)
             viewer.destroy()
+            os.replace(scratch, filename + ".h5")
     finally:
         # The gmsh import options are import-time scratch — meaningful only for
         # the createFromFile above. Clear the whole namespace so a value set by
@@ -155,7 +166,10 @@ def _from_gmsh(filename, comm=None, markVertices=False, useRegions=True, useMult
         # read as 2-D). Runs on success or failure.
         _clear_gmsh_import_options()
 
-    # Now we have an h5 file and we can hand this to _from_plexh5
+    # Now we have an h5 file and we can hand this to _from_plexh5. The barrier
+    # is what the atomic write above makes necessary AND sufficient: the other
+    # ranks must not look for the file before rank 0 has renamed it into place.
+    uw.mpi.barrier()
 
     return _from_plexh5(filename + ".h5", comm, return_sf=True)
 

src/underworld3/meshing/_mesh_files.py

@@ -0,0 +1,77 @@
+r"""Where generated mesh files are written, and how they are written.
+
+Building a mesh hands gmsh a file to write and then reads it back through
+PETSc, so the pair — ``<name>.msh`` and the ``<name>.msh.h5`` PETSc converts
+it to — is scratch shared between those two steps. It is not a cache: every
+construction regenerates both, and nothing checks whether they already exist.
+
+The name is derived from the mesh PARAMETERS, so two processes building the
+same geometry in one working directory choose the same name, and one can read
+a file the other is still writing (issue #563). Identical geometry is exactly
+the colliding case, which is why a parameter sweep or a parallel test run hits
+it and ordinary use does not.
+
+Two mechanisms make that safe, and both are needed:
+
+* the directory is settable per process through ``UW_MESH_CACHE_DIR``, so
+  independent jobs can be given somewhere of their own;
+* every write lands atomically, so a reader that shares a directory anyway
+  sees a complete file or no file, never a half-written one.
+"""
+import os
+from pathlib import Path
+
+import underworld3 as uw
+
+DEFAULT_MESH_FILE_DIR = ".meshes"
+
+
+def mesh_file_dir():
+    """The directory generated mesh files are written to.
+
+    ``UW_MESH_CACHE_DIR`` overrides the default ``.meshes``. Every rank of one
+    job must agree on it, so this reads the environment — inherited identically
+    by every rank — and never anything process-local such as the pid.
+    """
+    return Path(os.environ.get("UW_MESH_CACHE_DIR", DEFAULT_MESH_FILE_DIR))
+
+
+def mesh_file_path(basename):
+    """Full path for a generated mesh file, with its directory created.
+
+    Parameters
+    ----------
+    basename : str
+        The file's name, conventionally ``uw_<generator>_<parameters>.msh``.
+    """
+    directory = mesh_file_dir()
+    if uw.mpi.rank == 0:
+        directory.mkdir(parents=True, exist_ok=True)
+    return str(directory / basename)
+
+
+def _scratch_name(final):
+    """A process-unique sibling of ``final`` KEEPING ITS EXTENSION.
+
+    The extension has to survive: gmsh chooses its output format from it, so
+    writing to ``mesh.msh.1234.tmp`` would silently produce something that is
+    not a gmsh mesh.
+    """
+    final = Path(final)
+    return final.with_name(f"{final.stem}.{os.getpid()}.tmp{final.suffix}")
+
+
+def write_gmsh(filename):
+    """``gmsh.write``, landing atomically at ``filename``.
+
+    gmsh writes in place, so a concurrent reader can open a file that is still
+    being filled. Writing under a process-unique name and renaming makes the
+    appearance of the final name atomic — :func:`os.replace` is atomic within a
+    filesystem — so a reader sees either the previous complete file or the new
+    one.
+    """
+    import gmsh
+
+    scratch = _scratch_name(filename)
+    gmsh.write(str(scratch))
+    os.replace(scratch, filename)