Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 37 additions & 10 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,22 +140,23 @@ This keeps feature branches independent and makes cross-pollination of fixes str
**Use a worktree for any multi-file change** (docs cleanup, refactoring, features).
Multiple Claude sessions sharing one working directory will overwrite each other's work.

Worktrees share the main repo's pixi environment and PETSc build via symlinks —
there is one set of dependencies, not one per worktree. `./uw build` from inside
a worktree installs that worktree's source into the shared environment.
Each worktree gets its **own pixi environment** (isolated site-packages, own
compiled extensions). Only PETSc is shared via symlink (non-relocatable,
expensive to rebuild). `./uw build` from inside a worktree installs that
worktree's source into the worktree's own environment.

**Full documentation**: `docs/developer/guides/branching-strategy.md` (Git Worktrees section)

#### Creating and using a worktree

```bash
# Create — resets to development, sets up symlinks, names the branch
# Create — own .pixi env, shared PETSc, names the branch
./uw worktree create <name> # → feature/<name>
./uw worktree create <name> bugfix # → bugfix/<name>

# Work — drops you into a shell cd'd to the worktree
./uw worktree shell <name>
./uw build # builds from THIS source into the shared env
./uw build # builds from THIS source into THIS worktree's env
./uw test # runs tests
exit # leave

Expand All @@ -175,9 +176,8 @@ git checkout origin/<branch> -- path/to/file

#### Important: always build and run from inside the worktree

Because there is one shared environment, `./uw build` installs whichever source
tree you run it from. If you build from the main repo then run code expecting
worktree changes, the worktree edits will not be active. Always:
Each worktree has its own pixi environment. `./uw build` installs into the
environment of whichever worktree (or main repo) you run it from. Always:

1. `./uw worktree shell <name>` (or `cd` into the worktree)
2. `./uw build`
Expand Down Expand Up @@ -211,13 +211,40 @@ Underworld development team with AI support from [Claude Code](https://claude.co
### Rebuild After Source Changes
**After modifying source files, always run `./uw build`!**
- Underworld3 is installed as a package in the pixi environment
- Changes go to `.pixi/envs/default/lib/python3.12/site-packages/underworld3/`
- Verify with `uw.model.__file__`
- Changes go to `.pixi/envs/<env>/lib/python3.12/site-packages/underworld3/`
- Verify with `uw.__file__` (should show site-packages path, NOT `src/`)

**Note**: `./uw build` uses `--no-cache-dir` to prevent pip from reusing stale
wheels (UW3 is always version `0.0.0`). If you still suspect stale code, clean
the build directory: `rm -rf build/lib.* build/bdist.*` then rebuild.

### NEVER Use Editable Installs
**DO NOT use `pip install -e .` (editable/development mode)!**
This is a hard rule — there are no exceptions.

Editable installs create `.pth` files and `.so` symlinks in the source tree that:
- **Contaminate all pixi environments** sharing the same source directory
- **Break worktree isolation** (worktrees share pixi envs via symlinks)
- **Persist after uninstall** — stale `.pth` files redirect Python imports to `src/`
even after a proper `./uw build`, causing import errors or wrong library loading
- **Mix debug/release builds** — `.so` compiled against one PETSc arch get loaded
by environments expecting another, causing `dlopen` symbol errors

Always use `./uw build` which runs `pip install .` (non-editable). If `./uw build`
is not available, use `pixi run -e <env> pip install . --no-build-isolation --no-cache-dir`.

**Recovery from editable install contamination:**
```bash
# Remove stale .pth files from ALL environments
find .pixi/envs -name "__editable__*underworld*" -delete
# Remove .so from source tree (they belong in site-packages)
find src/underworld3 -name "*.so" -delete
# Clean build cache
rm -rf build/
# Rebuild properly
./uw build
```

### Test Quality Principles
**New tests must be validated before making code changes to fix them!**
- Validate test correctness before changing main code
Expand Down
1,042 changes: 1,012 additions & 30 deletions pixi.lock

Large diffs are not rendered by default.

24 changes: 24 additions & 0 deletions pixi.toml
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,27 @@ PETSC_ARCH = "petsc-4-uw-openmpi"
petsc-local-build = { cmd = "./build-petsc.sh", cwd = "petsc-custom" }
petsc-local-clean = { cmd = "./build-petsc.sh clean", cwd = "petsc-custom" }

# ============================================
# CUSTOM PETSC (AMR) — debug build
# ============================================
# Debug PETSc build (--with-debugging=1, -g -O0) for issue investigation.
# Uses the same conda deps as amr but points to a separate PETSC_ARCH
# so the optimised build is untouched.
#
# Setup:
# cd petsc-custom/petsc
# pixi run -e amr-debug python3 ./configure \
# --with-petsc-arch=petsc-4-uw-openmpi-debug --with-debugging=1 \
# --with-mpi-dir="$CONDA_PREFIX" --with-hdf5=0 \
# --download-mpich=0 --download-openmpi=0 --download-mpi4py=0 \
# --with-petsc4py=0 --with-x=0 --with-pragmatic=0 --with-slepc=0 \
# "--COPTFLAGS=-g -O0" "--CXXOPTFLAGS=-g -O0" "--FOPTFLAGS=-g -O0"
# make PETSC_DIR=$(pwd) PETSC_ARCH=petsc-4-uw-openmpi-debug all

[feature.amr-debug.activation.env]
PETSC_DIR = "$PIXI_PROJECT_ROOT/petsc-custom/petsc"
PETSC_ARCH = "petsc-4-uw-openmpi-debug"

# ============================================
# HPC CLUSTER FEATURE
# ============================================
Expand Down Expand Up @@ -317,6 +338,9 @@ amr = { features = ["amr"], solve-group = "amr" }
amr-runtime = { features = ["amr", "runtime"], solve-group = "amr" }
amr-dev = { features = ["amr", "runtime", "dev"], solve-group = "amr" }

# --- Debug PETSc (issue investigation, isolated from optimised build) ---
amr-debug = { features = ["amr", "amr-debug", "runtime", "dev"], solve-group = "amr" }

# --- Explicit MPICH (override for macOS, or when MPICH is required) ---
mpich = { features = ["conda-petsc-mpich"], solve-group = "mpich" }
mpich-dev = { features = ["conda-petsc-mpich", "runtime", "dev"], solve-group = "mpich" }
Expand Down
36 changes: 36 additions & 0 deletions src/underworld3/cython/petsc_compat.h
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,42 @@ PetscErrorCode UW_PetscDSViewBdWF(PetscDS ds, PetscInt bd)
return 1;
}

// Issue #96 fix: Force coordinate field creation on a DM and strip
// boundary labels from the coordinate DM so they don't cause MPI errors
// in DMCompleteBCLabels_Internal during lazy coordinate field recreation.
//
// Must be called AFTER createCoordinateSpace and AFTER labels are added.
// The coordinate field is created NOW (while we can clean the coord DM)
// and cached, preventing PETSc from lazily recreating it later.
PetscErrorCode UW_DMForceCoordinateField(DM dm)
{
DMField coordField;
DM cdm;
PetscInt numLabels, i;

PetscFunctionBeginUser;

// Force coordinate field creation (triggers DMCreateCoordinateField_Plex)
PetscCall(DMGetCoordinateField(dm, &coordField));

// Now strip non-essential labels from the coordinate DM
// (DMClone copied all of mesh.dm's labels, including boundary labels)
PetscCall(DMGetCoordinateDM(dm, &cdm));
PetscCall(DMGetNumLabels(cdm, &numLabels));
for (i = numLabels - 1; i >= 0; --i) {
const char *name;
PetscBool isDepth, isCelltype;
PetscCall(DMGetLabelName(cdm, i, &name));
PetscCall(PetscStrcmp(name, "depth", &isDepth));
PetscCall(PetscStrcmp(name, "celltype", &isCelltype));
if (!isDepth && !isCelltype) {
PetscCall(DMRemoveLabel(cdm, name, NULL));
}
}

PetscFunctionReturn(PETSC_SUCCESS);
}

// Set the time value on a DM. This is passed as `petsc_t` to all
// pointwise residual and Jacobian functions during assembly.
// PETSc stores this internally but petsc4py doesn't expose it.
Expand Down
1 change: 1 addition & 0 deletions src/underworld3/cython/petsc_extras.pxi
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ cdef extern from "petsc_compat.h":
PetscErrorCode UW_PetscDSViewBdWF(PetscDS, PetscInt)
PetscErrorCode UW_DMSetTime( PetscDM, PetscReal )
PetscErrorCode UW_DMPlexSetSNESLocalFEM( PetscDM, PetscBool, void *)
PetscErrorCode UW_DMForceCoordinateField(PetscDM)
PetscErrorCode UW_DMPlexComputeBdIntegral( PetscDM, PetscVec, PetscDMLabel, PetscInt, const PetscInt*, void*, PetscScalar*, void*)

cdef extern from "petsc.h" nogil:
Expand Down
16 changes: 16 additions & 0 deletions src/underworld3/cython/petsc_maths.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,22 @@ cdef extern from "petsc.h" nogil:
PetscErrorCode DMPlexComputeCellwiseIntegralFEM( PetscDM, PetscVec, PetscVec, void* )


def dm_force_coordinate_field(dm):
"""Force coordinate field creation and strip boundary labels from the
coordinate DM. Must be called after createCoordinateSpace and after
boundary labels have been added to mesh.dm.

Issue #96: DMClone (inside createCoordinateSpace) copies ALL labels from
mesh.dm to the coordinate DM. When DMPlexComputeBdIntegral later lazily
recreates the coordinate field, DMCompleteBCLabels_Internal fails with
MPI errors on the boundary labels. This function forces the coordinate
field to be created NOW and strips the labels so the cached field is used
instead of being lazily recreated.
"""
cdef DM c_dm = dm
CHKERRQ(UW_DMForceCoordinateField(c_dm.dm))


class Integral:
"""
The `Integral` class constructs the volume integral
Expand Down
36 changes: 24 additions & 12 deletions src/underworld3/discretisation/discretisation_mesh.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,22 +128,18 @@ def _from_plexh5(
if comm == None:
comm = PETSc.COMM_WORLD

viewer = PETSc.ViewerHDF5().create(filename, "r", comm=comm)
# Use createFromFile for a single-call load (issue #96: the separate
# topologyLoad + coordinatesLoad + labelsLoad pipeline leaves the
# coordinate DM in a state that DMPlexComputeBdIntegral cannot handle).
h5plex = PETSc.DMPlex().createFromFile(filename, interpolate=True, comm=comm)

# h5plex = PETSc.DMPlex().createFromFile(filename, comm=comm)
h5plex = PETSc.DMPlex().create(comm=comm)
sf0 = h5plex.topologyLoad(viewer)
h5plex.coordinatesLoad(viewer, sf0)
h5plex.labelsLoad(viewer, sf0)

# Do this as well
h5plex.setName("uw_mesh")
h5plex.markBoundaryFaces("All_Boundaries", 1001)

if not return_sf:
return h5plex
else:
return sf0, h5plex
return h5plex.getPointSF(), h5plex


class Mesh(Stateful, uw_object):
Expand Down Expand Up @@ -1144,11 +1140,25 @@ def nuke_coords_and_rebuild(

if PETSc.Sys.getVersion() <= (3, 20, 5) and PETSc.Sys.getVersionInfo()["release"] == True:
self.dm.projectCoordinates(self.petsc_fe)
elif hasattr(self.dm, "createCoordinateSpace"):
# Use createCoordinateSpace rather than setCoordinateDisc.
# setCoordinateDisc with a user-created FE leaves the coordinate
# dual space without proper point subspaces, causing
# DMPlexComputeBdIntegral to segfault/deadlock (issue #96).
# createCoordinateSpace builds the FE internally with correct
# subspace initialisation.
self.dm.createCoordinateSpace(self.degree, False, True)

Comment on lines +1144 to +1151

Copilot AI Apr 15, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the PETSc >3.20.5 path, createCoordinateSpace(...) is used and the previously-created self.petsc_fe is no longer referenced in this method. If it is truly only needed for the <=3.20.5 projectCoordinates() branch, consider creating it conditionally (or documenting why it must still be constructed) to avoid unnecessary FE creation and reduce confusion about which coordinate discretisation is actually in use.

Copilot uses AI. Check for mistakes.
# Issue #96 fix: Force coordinate field creation and strip
# boundary labels from the coordinate DM. createCoordinateSpace
# clears the coordinate field cache. Without this, BdIntegral
# lazily recreates the field by cloning mesh.dm (with boundary
# labels), causing DMCompleteBCLabels_Internal MPI errors.
from underworld3.cython.petsc_maths import dm_force_coordinate_field
dm_force_coordinate_field(self.dm)
elif PETSc.Sys.getVersion() >= (3, 24, 0):
# PETSc 3.24+ added 'localized' parameter (for DG coordinate spaces)
self.dm.setCoordinateDisc(disc=self.petsc_fe, localized=False, project=False)
else:
# PETSc 3.21-3.23: older signature without localized parameter
self.dm.setCoordinateDisc(disc=self.petsc_fe, project=False)

if verbose and uw.mpi.rank == 0:
Expand Down Expand Up @@ -1193,7 +1203,9 @@ def nuke_coords_and_rebuild(
self._search_lengths,
) = self._get_mesh_sizes()

self.dm.copyDS(self.dm_hierarchy[-1])
# Skip self-copy when hierarchy is trivial (issue #96 investigation)
if self.dm is not self.dm_hierarchy[-1]:
self.dm.copyDS(self.dm_hierarchy[-1])

if verbose and uw.mpi.rank == 0:
print(
Expand Down
52 changes: 39 additions & 13 deletions uw
Original file line number Diff line number Diff line change
Expand Up @@ -167,16 +167,41 @@ run_build() {
echo "$current_target" > "$petsc_marker"

echo " Building underworld3..."
# Determine the ACTUAL site-packages that Python will search.
# pip may resolve a different path (e.g. through worktree symlinks),
# so we ask Python directly and install with --target.
local site_packages
site_packages=$($PIXI run -e "$env" python -c "import site; print(site.getsitepackages()[0])" 2>/dev/null)
if [ -z "$site_packages" ]; then
echo -e "${YELLOW}Could not determine site-packages path${NC}"
exit 1
fi

# Remove any stale editable install markers (pip install -e leaves .pth files
# that redirect imports to the source tree, breaking environment isolation)
find "$site_packages" -maxdepth 1 -name "__editable__*underworld*" -delete 2>/dev/null

# Remove any stale .so from the source tree (left by editable installs)
find "$SCRIPT_DIR/src/underworld3" -name "*.so" -delete 2>/dev/null

# --no-cache-dir: UW3 is always version 0.0.0, so pip's wheel cache
# treats every build as "already cached" and silently reuses stale code.
# --force-reinstall: pip also skips reinstalling .py files if it thinks
# the package is already installed at the same version. This ensures
# all source files (not just compiled .pyx) are copied to site-packages.
$PIXI run -e "$env" pip install . --no-build-isolation --no-cache-dir --force-reinstall || {
# --target: install to where Python ACTUALLY looks, not where pip thinks.
# NEVER use -e (editable) — it breaks worktree and environment isolation.
$PIXI run -e "$env" pip install . --no-build-isolation --no-cache-dir \
--upgrade --target="$site_packages" || {
echo -e "${YELLOW}underworld3 build failed${NC}"
exit 1
}

# Verify the install is importable
$PIXI run -e "$env" python -c "import underworld3" 2>/dev/null || {
echo -e "${YELLOW}underworld3 installed but cannot be imported!${NC}"
echo " site-packages: $site_packages"
echo " Check for stale .pth files or .so mismatches."
exit 1
}

# Verify Python source files were actually installed.
# pip's wheel cache can silently serve stale .py files even with
# --no-cache-dir (version 0.0.0 problem). Compare checksums of key
Expand Down Expand Up @@ -833,7 +858,7 @@ COMMANDS
type-check Run mypy

Worktrees:
worktree create <name> [prefix] Create worktree (symlinks .pixi + PETSc)
worktree create <name> [prefix] Create worktree (own .pixi, shared PETSc)
worktree shell <name> Start shell in worktree directory
worktree list List worktrees with status
worktree remove <name> Remove worktree and branch
Expand Down Expand Up @@ -978,9 +1003,9 @@ run_update() {

# ── Worktree management ──────────────────────────────────────────────
#
# Worktrees share the main repo's pixi environment and PETSc build
# via symlinks. `./uw build` from inside a worktree installs the
# worktree's source into the shared env.
# Each worktree gets its OWN pixi environment (isolated site-packages).
# Only PETSc is shared via symlink (non-relocatable, expensive to rebuild).
# `./uw build` from inside a worktree installs into that worktree's env.

WORKTREE_ROOT="$SCRIPT_DIR/.claude/worktrees"

Expand Down Expand Up @@ -1058,15 +1083,16 @@ worktree_create() {
echo -e " ${GREEN}✓${NC} .pixi-env copied ($(cat "$main_repo/.pixi-env"))"
fi

# Install pixi environment (own copy, not symlinked)
# Install pixi environment (own copy — NEVER symlink)
local env=$(cat "$wt_path/.pixi-env" 2>/dev/null || echo "runtime")
echo " Installing pixi environment ($env) — this may take a moment..."
(cd "$wt_path" && $PIXI install -e "$env" 2>/dev/null) && \
echo -e " ${GREEN}✓${NC} .pixi/ installed (isolated)" || {
echo -e " ${YELLOW}pixi install failed — falling back to symlink${NC}"
rm -rf "$wt_path/.pixi"
ln -s "$main_repo/.pixi" "$wt_path/.pixi"
echo -e " ${YELLOW}✓${NC} .pixi → main repo (shared fallback)"
echo -e " ${RED}pixi install failed for worktree${NC}"
echo -e " The worktree requires its own pixi environment."
echo -e " Fix the pixi install error above, then run:"
echo -e " cd $wt_path && pixi install -e $env"
exit 1
}

# 4b. Install petsc4py for AMR environments
Expand Down
Loading