diff --git a/.github/actions/install/action.yml b/.github/actions/install/action.yml index ddd9d583b1..40bcfee39b 100644 --- a/.github/actions/install/action.yml +++ b/.github/actions/install/action.yml @@ -154,6 +154,7 @@ runs: EXTRA_PIP_FLAGS='--no-build-isolation' elif [ ${{ inputs.base_ref }} = 'release' ]; then EXTRA_PIP_FLAGS='' + export PIP_BUILD_CONSTRAINT=constraints.txt else echo "Unrecognised 'base_ref' input: '${{ inputs.base_ref }}" exit 1 diff --git a/AGENTS.md b/AGENTS.md index ffc2ca2295..9505d47225 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,195 +1,153 @@ # Firedrake Firedrake is an automated system for the portable solution of partial differential equations using -the finite element method (FEM). The codebase is primarily Python, relying heavily on code generation -and high-performance C backends to achieve scalability and speed. - -Firedrake's full contribution process is documented at -[Contributing to Firedrake](https://firedrakeproject.org/contribute.html). In short, for AI-assisted -contributions: declare that AI was used and which tool; a human must lead the PR, understand every -change, and answer reviewer questions themselves rather than relaying them to the AI; the code must -have been run locally to confirm it works; and AI should not be used to close issues labelled -'good first issue'. The full, authoritative conditions are in the -[AI contribution policy](https://github.com/firedrakeproject/firedrake/wiki/AI-contribution-policy). +the finite element method (FEM). The codebase is primarily Python, with code generation and +high-performance C backends for scalability and speed. ## Project Architecture -Firedrake solves variational problems discretized with finite elements through a coordinated -toolchain: - -* **PETSc:** Firedrake relies on PETSc (specifically `DMPlex`) for scalable mesh management and - parallel data distribution. PETSc's `PC`/`KSP`/`SNES` are used extensively as the underlying - preconditioners and linear/nonlinear solvers, respectively. -* **UFL (Unified Form Language):** Users symbolically specify their variational problems and forms - using UFL. -* **TSFC (Two-Stage Form Compiler):** TSFC automatically generates highly optimized C code to assemble - the UFL integrals, in two stages: - 1. **Lowering to GEM:** TSFC lowers UFL into the GEM tensor language. GEM represents expressions over - quadrature points involving compile-time pre-tabulated basis functions provided by **FInAT** and - **FIAT**. - 2. **Lowering to Loopy:** The GEM expressions are then lowered into **loopy** kernels. -* **PyOP2:** Finally, the generated loopy kernels are wrapped and executed by PyOP2, which handles the - parallel execution of loops over mesh cells and facets. +Firedrake's toolchain, in order: + +* **PETSc:** `DMPlex` manages meshes and parallel data distribution. `PC`/`KSP`/`SNES` are the + preconditioners and linear/nonlinear solvers. +* **UFL (Unified Form Language):** Users write variational problems and forms in UFL. +* **TSFC (Two-Stage Form Compiler):** Generates optimized C to assemble UFL integrals, in two stages: + 1. **Lowering to GEM:** TSFC lowers UFL into GEM, a tensor language for expressions over quadrature + points, using basis functions pre-tabulated by **FInAT** and **FIAT**. + 2. **Lowering to Loopy:** GEM lowers into **loopy** kernels. +* **PyOP2:** Wraps and executes the loopy kernels, parallelizing loops over mesh cells and facets. ## Core Working Rules -* **Mathematical Root Causes:** Bug fixes must address the underlying core mathematical or - architectural issue. Do not merely patch particular failing test cases or edge cases. -* **Generality Over Complexity:** Avoid increasing code complexity with complicated bookkeeping or - special-case logic. Firedrake relies on the mathematical generality of finite elements. -* **Unified Abstractions:** Proper Firedrake code avoids branching on the wide range of discretizations - (e.g., cell type, polynomial degree, or finite element family) or execution states (serial vs. MPI - parallel). Rely on UFL, TSFC, PyOP2, and PETSc abstractions to handle these variations - transparently. -* **Preserve Style:** Preserve Firedrake style and naming conventions. Keep edits minimal and local to - the requested change. Match existing patterns in the package you are modifying. -* **Avoid Duplication:** Avoid unnecessary code duplication. Prefer reusing or extending nearby logic - when it keeps behavior clear and local. Do not add speculative abstractions or broad refactors unless - explicitly requested. -* **Do Not Trust Memorized API Shapes:** Firedrake, UFL, and PETSc/petsc4py APIs change over time — - properties become methods, arguments get renamed, call signatures get deprecated. An LLM's trained - knowledge reflects a snapshot that may already be stale, and will confidently reproduce the old, - no-longer-correct form (e.g. calling a now-method as a bare attribute, or vice versa). Before calling - an API you have not just seen used in this codebase, verify its actual current signature by reading - the installed Firedrake/UFL/PETSc source rather than relying on memorized patterns. -* **Document The Present, Not The Past:** When fixing code that was wrong, do not leave comments or - prose explaining what the removed, incorrect approach used to do or why it was wrong. Keep comments - and documentation focused on the current, correct code; a reader should never need the history of - what used to be there to understand why the present code is right. +* **Mathematical Root Causes:** Fix the underlying mathematical or architectural cause. Do not patch + individual failing test cases. +* **Generality Over Complexity:** Rely on the mathematical generality of finite elements. Do not add + special-case bookkeeping or branching. +* **Unified Abstractions:** Do not branch on cell type, polynomial degree, element family, or serial + vs. MPI-parallel execution. Use the UFL/TSFC/PyOP2/PETSc abstraction that already handles it — see + Anti-Patterns. +* **Preserve Coding Style:** Match the naming and patterns of the package you are editing. Keep edits minimal + and local to the requested change. However, do not match the terse, telegraphic style of existing + comments and docstrings. +* **Avoid Duplication:** Reuse or extend nearby logic instead of duplicating it. Do not add speculative + abstractions or broad refactors unless asked. +* **Do Not Trust Memorized API Shapes:** Read a Firedrake, UFL, or PETSc/petsc4py API's current + signature from the installed source before calling it, unless you have just seen it used in this + codebase. +* **Document The Present, Not The Past:** Do not describe a removed or rejected approach in a comment + or docstring. Document only what the current code does. ## Coding Style And Conventions -* **Class Attributes:** Every attribute a class can hold must be declared in one visible place, either - initialized in the constructor (`__init__`) or, for state that is expensive or unnecessary to compute - eagerly, declared as a `functools.cached_property`. Avoid discovering an attribute's existence via - `hasattr`/`setattr`/`getattr` scattered across methods — laziness is fine, ad hoc laziness is not. -* **No Python Mesh Loops:** The Firedrake style strictly avoids using Python `for` loops to iterate - over degrees of freedom (DoFs) or cells in a mesh. -* **Prefer Code Generation/PETSc:** All mesh-level or DoF-level operations must be implemented using - PyOP2-driven kernels or DMPlex operations. These should be accessed either through `petsc4py` or - Firedrake's internal Cython wrappers. -* **NumPy Is Fine, Repeatedly Touching Whole Arrays Is Not:** NumPy is the right tool for index - computations, small metadata configurations, and vectorized pre/post-processing. The anti-pattern is - not "using NumPy" but iterating a large array element-by-element (in a Python `for` loop) or - otherwise touching the same whole array repeatedly outside of a single vectorized expression — that - is what defeats NumPy's own performance model, on top of bypassing PyOP2/code-generation for - mesh-bound data. -* **Docstrings Are Always `numpydoc`:** Every docstring you write or touch — public API, private helper, - Cython function in `firedrake/cython/*.pyx`, test helper — must be `numpydoc`, using its section - headings (`Parameters`, `Returns`, `Raises`, `Notes`). Never write the old Sphinx field-list style - (`:arg x:`, `:param x:`, `:returns:`, `:rtype:`) in new or edited code, and do not copy it from the - surrounding file: much of Firedrake predates the convention, so matching the neighbouring docstrings - is precisely the wrong instinct — this is the one place where "preserve the existing style" does not - apply. Being private, internal, or compiled is not an excuse to skip the docstring, to downgrade its - format, or to leave the arguments undocumented: give every parameter and every return value its - `numpydoc` entry, however small the helper. -* **Type Hints:** New code should include type hints on function/method signatures. -* **Demos Are Literate Programs:** `pylit` converts each `demos//.py.rst` into a `.py` that - `tests/firedrake/demos/test_demos_run.py` executes, so prose and code must stay in step. A paragraph - ending in `::` makes the indented block after it *executable*; a `.. code-block:: python` directive is - excluded from that rule, so its snippet renders in the docs but never runs. Prefer `::` — reach for - the directive only for an illustrative fragment naming things the demo never defines. +* **Class Attributes:** Declare every attribute in `__init__`, or as a `functools.cached_property` for + state that is expensive to compute eagerly. Do not discover an attribute via + `hasattr`/`setattr`/`getattr` — see Anti-Patterns. +* **No Python Mesh Loops:** Never iterate over degrees of freedom or cells with a Python `for` loop — + see Anti-Patterns. +* **Prefer Code Generation/PETSc:** Implement mesh-level or DoF-level operations through PyOP2-driven + kernels or DMPlex, via `petsc4py` or Firedrake's Cython wrappers. +* **NumPy For Vectorized Work Only:** Use NumPy for index computation and vectorized pre/post-processing. + Do not iterate a large array element-by-element, or touch the same whole array repeatedly outside one + vectorized expression. +* **Docstrings and Type Hints:** The codebase is mid-migration and inconsistently documented and typed. + All public-facing APIs that you touch must be updated to `numpydoc`-style. + Add type hints to new function/method signatures. ## Testing Requirements -* **Pull Requests:** All PRs must include comprehensive tests demonstrating that the new feature works - or the bug is fixed. -* If behavior changes, update the relevant test blocks and ensure that parallel runs (MPI) yield - correct and identical mathematical results to serial runs. -* Keep tests targeted. Add or update the narrowest test that proves the behavior you changed. -* Do not create new test files for this. Add the new test(s) to the existing test file(s) that already - cover the feature or module being changed. +* Add tests that demonstrate the new feature or bug fix, in the existing test file for that module. +* When behavior changes, update the affected tests and confirm parallel (MPI) runs match serial + results. +* Add or update the narrowest test that proves the change. ## Pull Request Expectations -* All changes are expected to arrive through GitHub Pull Requests. -* Keep diffs reviewable and focused. -* Before concluding work, ensure `make srclint` passes, and verify that the relevant subset of the - pytest test suite succeeds locally. +* All changes land through GitHub pull requests. Keep diffs focused. +* Before requesting review: `make srclint`, ensure the relevant test subset is green, and read the + [pre-submission checklist](https://firedrakeproject.org/contribute.html#pre-submission-checklist) in + `docs/source/contribute.rst`. +* Contributions assisted by AI must state which tool was used and apply the `LLM used` pull request label. ## Development Toolchain ### Environment Setup -* **Editable installs across the stack:** A bug can live in Firedrake or in any of its component - packages (PETSc, petsc4py, UFL, FIAT, FInAT, TSFC, PyOP2, loopy). Follow the - ["Editing subpackages"](https://firedrakeproject.org/install.html#editing-subpackages) instructions - in the install docs to get a component installed in editable mode so source edits take effect without - reinstalling, and check which branch/commit of each component is actually active before assuming a - fix belongs in Firedrake itself. -* **`petsc4py`/PETSc version skew:** `petsc4py` is a compiled extension built against one specific - PETSc checkout. If you switch the PETSc branch/commit underneath an existing venv (e.g. to bisect a - PETSc-side issue) without rebuilding `petsc4py` against it, `import firedrake` fails with a confusing - `undefined symbol: ...` error from `petsc4py`'s `.so` — not a Firedrake traceback, and easy to - misattribute to whatever you were just changing. Rebuild `petsc4py` (and re-run - `pip install --no-build-isolation -e .` for it) after switching PETSc, rather than debugging the - symptom. -* **Caching:** Generated TSFC kernels and compiled PyOP2 code are cached on disk, under - `FIREDRAKE_TSFC_KERNEL_CACHE_DIR`/`PYOP2_CACHE_DIR`. These are not pre-set shell variables — do not - expect `echo $PYOP2_CACHE_DIR` to show anything. `firedrake.configuration.setup_cache_dirs()` sets - them in-process, defaulting to `$VIRTUAL_ENV/.cache/{tsfc,pyop2}`, as one of the first things - `import firedrake` does (right after PETSc initialization, before PyOP2 loads) unless you already - exported them yourself beforehand. This also means that if PETSc initialization itself fails (e.g. - the version-skew symptom above), these variables never get set at all. If a code-generation change - does not seem to take effect, or you suspect a stale kernel, run `firedrake-clean` before re-testing - (it prints the actual paths in use). -* **Smoke test after install/rebuild:** `firedrake-check` runs a small grouped-by-process-count subset - of the regression suite; use it to sanity-check an environment before investing time in a full test - run. +* **Editable installs:** Install PETSc, petsc4py, UFL, FIAT, FInAT, TSFC, PyOP2, and loopy in editable + mode (see ["Editing subpackages"](https://firedrakeproject.org/install.html#editing-subpackages)) so + source edits take effect without reinstalling. Check each component's active branch/commit before + assuming a fix belongs in Firedrake. +* **`petsc4py`/PETSc version skew:** Rebuild `petsc4py` (`pip install --no-build-isolation -e .`) after + switching the PETSc branch/commit under an existing venv. A stale `petsc4py` fails `import firedrake` + with an `undefined symbol: ...` error, not a Firedrake traceback. +* **Caching:** TSFC kernels and PyOP2 code are cached under + `FIREDRAKE_TSFC_KERNEL_CACHE_DIR`/`PYOP2_CACHE_DIR` (default `$VIRTUAL_ENV/.cache/{tsfc,pyop2}`), set + by `firedrake.configuration.setup_cache_dirs()` on `import firedrake`. Run `firedrake-clean` if a + change to the code generator does not take effect. +* **Smoke test:** `firedrake-check` runs a process-count-grouped subset of the regression suite; use it + before a full run. ### Testing -* **Parallel tests:** Tests that must run under MPI are marked `@pytest.mark.parallel` (optionally - `@pytest.mark.parallel(nprocs=N)` or `@pytest.mark.parallel([1, 3])` for multiple process counts), via - the `mpi-pytest` plugin. Plain `pytest test_foo.py` does exercise them: for each parallel test it - self-forks an `mpiexec` subprocess with the right `nprocs`, one test at a time, which is slow and - produces one nested pytest report per test. To instead run every `nprocs=3` test in `test_foo.py` - together, directly under a single outer `mpiexec`, filter on the `parallel[match]` marker that the - plugin attaches to tests whose `nprocs` equals the launched communicator size: +* **Parallel tests:** Tests that need MPI are marked `@pytest.mark.parallel` (`nprocs=N`, or a list for + multiple counts), run via the `mpi-pytest` plugin. Plain `pytest test_foo.py` self-forks one + `mpiexec` subprocess per parallel test, one nested report each. Run every test at a given `nprocs` + together, under one outer `mpiexec`, filtered on `parallel[match]`: ```bash mpiexec -n 3 python -m pytest -m "parallel[match]" test_foo.py ``` - Tests requiring a different `nprocs` are collected but skipped (not run) by this invocation; do not - conclude a parallel code path is untested just because a plain, unmarked `pytest` run was green. -* **Splitting for CI:** `firedrake-run-split-tests` shards the suite by process count for CI; look at - it (and `.github/workflows/pr.yml`/`core.yml`) if a failure only reproduces in CI and not locally. + Tests at other `nprocs` are collected but skipped. A green unmarked `pytest` run is not evidence + that the parallel tests passed. +* **Splitting for CI:** `firedrake-run-split-tests` shards the suite by process count for CI. Check it + and `.github/workflows/pr.yml`/`core.yml` if a failure reproduces only in CI. * **Narrow reproduction first:** Run the single failing test node (`pytest path::test_name -k ...`) - before the full module; the suite is large and full-module reruns are slow to iterate against. + before the full module. ### Debugging -* **Generated kernels (niche, rarely needed):** By default, generated C is compiled optimized and - without debug symbols, so a debugger attached to the Python process cannot meaningfully step through - it. Set `PYOP2_DEBUG=1` to compile with `-O0 -g` instead, which is the prerequisite for using - `gdb`/`cgdb` on the compiled kernel at all. -* **Cross-rank code-generation mismatches:** If a parallel run raises `CompilationError: Generated code - differs across ranks`, the mismatching per-rank source is dumped under - `/mismatching-kernels/src-rank*.c`. Diffing the two sources only tells you *what* differs; - the actual fix is almost always upstream of that, in whatever Python-level parameter or branch is - computed differently per rank and fed into code generation (e.g. a rank-local decision that should be - a collective/global one) — make that decision the same on every rank, rather than patching the - generated source or the difference itself. -* **Parallel deadlocks (niche, rarely needed):** `PYOP2_SPMD_STRICT=1` adds barriers around calls - marked `@collective` and around cache access, trading overhead for a much narrower failure point when - ranks disagree about control flow. -* **Logging:** `firedrake.logging.set_log_level()` (or the `PYOP2_LOG_LEVEL` environment variable) - raises verbosity of Firedrake's/PyOP2's own logger, independent of PETSc's `-log_view`/`-info`. -* **PETSc-level diagnostics:** Since the linear/nonlinear solve ultimately runs through petsc4py, - standard PETSc options (`-ksp_view`, `-snes_view`, `-ksp_monitor`, `-log_view`, `-start_in_debugger`) - can be passed through Firedrake's `solver_parameters` or the command line exactly as in a plain PETSc +* **Generated kernels:** Set `PYOP2_DEBUG=1` to compile generated C with `-O0 -g`, needed for + `gdb`/`cgdb` on a compiled kernel. +* **Mismatches when ranks generate different code:** `CompilationError: Generated code differs across + ranks` dumps the mismatching per-rank source under `/mismatching-kernels/src-rank*.c`. Fix + the Python-level value that is computed differently per rank and that feeds into code generation, not + the generated source. +* **Parallel deadlocks:** `PYOP2_SPMD_STRICT=1` adds barriers around `@collective` calls and + cache access, to narrow down where ranks disagree on control flow. +* **Logging:** `firedrake.logging.set_log_level()` (or `PYOP2_LOG_LEVEL`) sets Firedrake/PyOP2 log + verbosity, independent of PETSc's `-log_view`/`-info`. +* **PETSc-level diagnostics:** Pass PETSc options (`-ksp_view`, `-snes_view`, `-ksp_monitor`, + `-log_view`, `-start_in_debugger`) through `solver_parameters` or the command line, as in any PETSc application. ### Reproducible Environments -* **Docker:** Pull one of the published images from - [Docker Hub](https://hub.docker.com/u/firedrakeproject) (e.g. `firedrakeproject/firedrake:latest`, - or `:dev-main`/`:dev-release` for the latest commit on each branch — see the - [install docs](https://firedrakeproject.org/install.html#docker)) to rule out "works on my machine" - environment drift before chasing a hard-to-reproduce bug. +* **Docker:** Pull a published image from + [Docker Hub](https://hub.docker.com/u/firedrakeproject) (`firedrakeproject/firedrake:latest`, or + `:dev-main`/`:dev-release` — see the + [install docs](https://firedrakeproject.org/install.html#docker)) to rule out environment drift + before chasing a hard-to-reproduce bug. + +## Grammar & Style Rules for Technical Prose + +Write as an expert technical writer addressing a peer (a mathematician or software engineer). +Use ASD-STE100. Write clear, complete sentences rather than grammatically convoluted shortcuts. +All comments, docstrings, and documentation must adhere to the following standards: + +* **Active Verbs Over Noun-Stacking:** Rephrase to avoid stacking words that double as nouns, verbs, or adjectives. + - **WRONG:** `# Process boundary facet normal orientation sign correction.` + - **RIGHT:** `# Flips boundary facets so their normals point outside the mesh.` + +* **Explicit Relative Pronouns:** Never drop pronouns like `that`, `which`, or `where` to condense sentences. + - **WRONG:** `# Function updates tensor values modified during solve step.` + - **RIGHT:** `# Updates tensor values that were modified during the solve step.` + +* **Subject-Verb Alignment:** Ensure that introductory prepositional phrases modify the actual grammatical +subject of the main clause. Avoid dangling modifiers. + - **WRONG**: `# Using the tangent linear model, $O(M)$ solves are needed.` + - **RIGHT**: `# The tangent linear approach requires $O(M)$ solves.` ## Anti-Patterns -These must be avoided when writing code, and flagged when reviewing it. +Each pattern below is a WRONG/RIGHT pair to read. ### Branching On Discretization Or Execution State @@ -218,10 +176,9 @@ def stable_timestep(mesh, velocity, cfl=0.5): return dt ``` -RIGHT — `CellDiameter` is defined uniformly for every cell type, so the branch on `cellname()` does -nothing but duplicate the same call. A per-cell timestep comes from interpolating into a `DG0` space, -and its global minimum from a PETSc `Vec`'s own collective `min()`, called unconditionally by every -rank rather than hand-rolled behind a `mesh.comm.rank` guard: +RIGHT — `CellDiameter` is defined uniformly for every cell type, so the branch on `cellname()` +duplicates the same call. A per-cell timestep comes from interpolating into a `DG0` space, and its +global minimum from a PETSc `Vec`'s own collective `min()`, called unconditionally by every rank: ```python def stable_timestep(mesh, velocity, cfl=0.5): @@ -252,10 +209,9 @@ class ResidualMonitor: print(f"iteration {it}: |F| = {norm(self._work)}") ``` -RIGHT — Laziness itself is fine — allocating a `Function` is not free, and this monitor may never be -attached to a solve — but express it with `functools.cached_property` rather than ad hoc -`hasattr`/`setattr`. The attribute is declared once, in the class body, and is computed and memoized -automatically on first access: +RIGHT — Express the laziness with `functools.cached_property` instead of ad hoc +`hasattr`/`setattr`. The attribute is declared once, in the class body, computed and memoized on +first access: ```python from functools import cached_property @@ -305,17 +261,16 @@ class KSPWrapper: self._ksp.solve(b, x) ``` -This is exactly the pattern used by `PCSNESBase` (`firedrake/preconditioners/base.py`), the base class -every Firedrake `PCBase`/`SNESBase` preconditioner inherits: its `__init__` sets -`self.initialized = False`, and `setUp()` dispatches to `initialize()` or `update()` based on that flag -rather than probing for the presence of state built by `initialize()`. A boolean records intent and is -trivially greppable; `hasattr` is indistinguishable from "I forgot to initialize this" until it fails. +`PCSNESBase` (`firedrake/preconditioners/base.py`), the base class that every +`PCBase`/`SNESBase` preconditioner inherits, uses exactly this pattern: `__init__` sets +`self.initialized = False`, and `setUp()` dispatches to `initialize()` or `update()` on that flag. A +boolean is greppable; `hasattr` is indistinguishable from a forgotten initialization until it fails. ### Python-Level Looping Over Mesh-Bound Array Data -WRONG — Pulling mesh/DoF data into a Python `for` loop, whether or not NumPy is involved. The problem -is the loop, not NumPy: a genuinely vectorized NumPy expression over the same array would be fine, but -would still bypass PyOP2/code-generation for anything mesh-bound: +WRONG — Pulling mesh/DoF data into a Python `for` loop. The problem is the loop, not NumPy — a +vectorized NumPy expression over the same array is fine, but still bypasses PyOP2/code-generation for +mesh-bound data: ```python # Anti-pattern: Python-level loop over mesh coordinates instead of a code-generated kernel @@ -349,10 +304,10 @@ non-variational, per-DoF transform), there are two sanctioned escape hatches, in par_loop((domain, instructions), dx, {"A": (A, RW), "B": (B, READ)}) ``` -2. A compiled Cython loop over the raw DoF array, following the same pattern Firedrake's own - `firedrake/cython/` wrappers use for mesh-topology bookkeeping (see below), for the rare case where - even a `par_loop` kernel is too restrictive (e.g. the transform needs a general-purpose C library - call that loopy cannot express): +2. A compiled Cython loop over the raw DoF array — the same pattern that Firedrake's own + `firedrake/cython/` wrappers use for bookkeeping on mesh topology (see below) — for the rare case + where a `par_loop` kernel cannot express the transform (e.g. it needs a general-purpose C library + call): ```cython # heavy_math.pyx, compiled ahead of time -- not a plain Python loop @@ -367,16 +322,10 @@ non-variational, per-DoF transform), there are two sanctioned escape hatches, in called from Python as `apply_heavy_math(mesh.coordinates.dat.data)`. -Compute-heavy operations that could otherwise be code-generated bypass Firedrake's parallelization, -cache-optimization, and MPI scaling capabilities, acting as massive performance bottlenecks. - -This rule is about Python-level loops. Explicit loops over mesh entities (cells, facets, closures) are -the norm, not an exception, inside Firedrake's own Cython wrappers in `firedrake/cython/` -(`dmcommon.pyx`, `extrusion_numbering.pyx`, `mgimpl.pyx`, `patchimpl.pyx`, ...), which exist precisely -to implement mesh-topology bookkeeping that has no UFL/TSFC representation — e.g. `create_cell_closure()` -in `dmcommon.pyx` loops `for c in range(cStart, cEnd)` to build the FIAT-ordered closure map that later -code generation depends on. These loops are compiled and typed (`cdef`/`PetscInt`, -`@cython.boundscheck(False)`), operating directly on DMPlex point ranges rather than interpreted Python -objects — that combination, not mere placement in a `.pyx` file, is what makes them acceptable. Do not -use this as license to write a plain Python loop over `.dat.data` and call it fine because "Firedrake -has C-level loops elsewhere." +This rule is about Python-level loops. Firedrake's own Cython wrappers in `firedrake/cython/` +(`dmcommon.pyx`, `extrusion_numbering.pyx`, `mgimpl.pyx`, `patchimpl.pyx`, ...) loop over mesh entities +routinely, for bookkeeping on mesh topology with no UFL/TSFC representation — e.g. `create_cell_closure()` +in `dmcommon.pyx` loops `for c in range(cStart, cEnd)` to build the closure map that code generation +depends on. These loops are compiled and typed (`cdef`/`PetscInt`, `@cython.boundscheck(False)`), on +DMPlex point ranges, not interpreted Python objects. A plain Python loop over `.dat.data` is not fine +merely because "Firedrake has C-level loops elsewhere." diff --git a/AUTHORS.rst b/AUTHORS.rst index 71fa431570..e2fa02db68 100644 --- a/AUTHORS.rst +++ b/AUTHORS.rst @@ -58,6 +58,8 @@ Ed Bueler Henrik Buesing +Anastasia Chanbour + Justin Chang Cyrus Cheng diff --git a/docs/Makefile b/docs/Makefile index c26010b389..775e1ad3ca 100644 --- a/docs/Makefile +++ b/docs/Makefile @@ -103,7 +103,7 @@ copy_notebooks: source/element_list.csv: source/element_list.py cd source; python3 element_list.py -source/team.rst: source/team2.py +source/team.rst: source/team2.py source/team.ini cd source; python3 team2.py; mv AUTHORS.rst ../.. source/apt_deps.txt: diff --git a/docs/source/firedrake_19.rst b/docs/source/firedrake_19.rst index b67ac14cf5..d537b8053a 100644 --- a/docs/source/firedrake_19.rst +++ b/docs/source/firedrake_19.rst @@ -58,8 +58,8 @@ presenters. * Colin Cotter: `Time (integrator) parallel exponential integration and phase-averaging for geophysical fluid dynamics <_static/firedrake19-slides/cotter.pdf>`__ -* Matt Knepley: `Understanding multivariate computation using the - Kolmogorov superposition theorem `_ +* Matt Knepley: Understanding multivariate computation using the + Kolmogorov superposition theorem * Lawrence Mitchell: `PCPATCH: topological construction of multigrid relaxation methods <_static/firedrake19-slides/mitchell.pdf>`__ * Joe Wallwork: `Anisotropic goal-oriented mesh adaptation in diff --git a/docs/source/images/anastasiachanbour.jpeg b/docs/source/images/anastasiachanbour.jpeg new file mode 100644 index 0000000000..f7a2fcabc9 Binary files /dev/null and b/docs/source/images/anastasiachanbour.jpeg differ diff --git a/docs/source/team.ini b/docs/source/team.ini index ff08978729..498583cb90 100644 --- a/docs/source/team.ini +++ b/docs/source/team.ini @@ -50,6 +50,7 @@ Daiane I. Dolci: Joshua Hope-Collins: https://www.imperial.ac.uk/people/joshua.hope-collins13 Umberto Zerbinati: https://www.uzerbinati.eu Leo Collins: +Anastasia Chanbour: [inactive-team] Lawrence Mitchell: https://www.wence.uk/ diff --git a/firedrake/assemble.py b/firedrake/assemble.py index 6dd7c4f03f..ffa8ec01a7 100644 --- a/firedrake/assemble.py +++ b/firedrake/assemble.py @@ -1074,7 +1074,14 @@ def parloops(self, tensor): if hasattr(self, "_parloops"): for (lknl, _), parloop in zip(self.local_kernels, self._parloops): data = self._as_pyop2_type(tensor, lknl.indices) - parloop.arguments[0].data = data + if isinstance(data, op2.Global): + # In parloops we swap out globals with private ones so + # increments don't double add. The right attribute to swap + # out here is therefore reduced_globals instead of arguments. + tmp = parloop.arguments[0].data + parloop.reduced_globals[tmp] = op2.GlobalParloopArg(data) + else: + parloop.arguments[0].data = data else: # Make parloops for one concrete output tensor and cache them. diff --git a/firedrake/supermeshing.py b/firedrake/supermeshing.py index 6d2638e3a3..ff3ff04db8 100644 --- a/firedrake/supermeshing.py +++ b/firedrake/supermeshing.py @@ -4,6 +4,7 @@ import pathlib import libsupermesh import petsctools +import rtree from firedrake.cython.supermeshimpl import assemble_mixed_mass_matrix as ammm, intersection_finder from firedrake.mg.utils import get_level @@ -462,9 +463,14 @@ def likely(cell_A): libsupermesh_dir = pathlib.Path(libsupermesh.get_include()).parent.absolute() dirs = petsctools.get_petsc_dirs() + (libsupermesh_dir,) - includes = ["-I%s/include" % d for d in dirs] - libs = ["-L%s/lib" % d for d in dirs] - libs = libs + ["-Wl,-rpath,%s/lib" % d for d in dirs] + ["-lpetsc", "-lsupermesh"] + includes = [f"-I{d}/include" for d in dirs] + libs = [ + *[f"-L{d}/lib" for d in dirs], + *[f"-Wl,-rpath,{d}/lib" for d in dirs], + "-lpetsc", + "-lsupermesh", + str(pathlib.Path(rtree.core.rt._name).absolute()), # libspatialindex.so + ] dll = load( supermesh_kernel_str, "c", cppargs=includes, diff --git a/pyproject.toml b/pyproject.toml index 435cccc87c..0c942f8a82 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -27,7 +27,8 @@ dependencies = [ "h5py>3.12.1", "firedrake-rtree>=2026.2.0", "immutabledict", - "libsupermesh>=2026.0", + # TODO RELEASE + "libsupermesh @ git+https://github.com/firedrakeproject/libsupermesh.git@main", "loopy>2024.1", "numpy", "packaging", @@ -41,6 +42,7 @@ dependencies = [ "pycparser", "pytools[siphash]", "requests", + "rtree", "scipy", "sympy", "islpy<2026.2", @@ -159,7 +161,8 @@ docker = [ # Used in firedrake-vanilla container requires = [ "Cython>=3.0", "firedrake-rtree>=2026.2.0", - "libsupermesh>=2026.0", + # TODO RELEASE + "libsupermesh @ git+https://github.com/firedrakeproject/libsupermesh.git@main", "mpi4py>3; python_version >= '3.13'", "mpi4py; python_version < '3.13'", "numpy", @@ -169,6 +172,7 @@ requires = [ "pkgconfig", "pybind11", "setuptools>=77.0.3", + "rtree", ] build-backend = "setuptools.build_meta" diff --git a/requirements-build.txt b/requirements-build.txt index 5e6ffbef0f..89f8a85c93 100644 --- a/requirements-build.txt +++ b/requirements-build.txt @@ -1,7 +1,8 @@ # Core build dependencies (adapted from pyproject.toml) Cython>=3.0 firedrake-rtree>=2026.2.0 -libsupermesh>=2026.0 +# TODO RELEASE +libsupermesh @ git+https://github.com/firedrakeproject/libsupermesh.git@main mpi4py>3; python_version >= '3.13' mpi4py; python_version < '3.13' numpy @@ -9,6 +10,7 @@ pkgconfig petsctools pybind11 setuptools>=77.0.3 +rtree # Transitive build dependencies hatchling diff --git a/setup.py b/setup.py index 2042d67e2f..e1e9ebe0b1 100644 --- a/setup.py +++ b/setup.py @@ -10,6 +10,7 @@ import libsupermesh import firedrake_rtree +import rtree import numpy as np import pybind11 import petsctools @@ -138,6 +139,19 @@ def __getitem__(self, key): ], ) +# libspatialindex +# example: +# gcc -I/rtree/include +# gcc /rtree.libs/libspatialindex.so -Wl,-rpath,$ORIGIN/../../Rtree.libs +libspatialindex_so = Path(rtree.core.rt._name).absolute() +spatialindex_ = ExternalDependency( + include_dirs=[rtree.finder.get_include()], + extra_link_args=[str(libspatialindex_so)], + runtime_library_dirs=[ + os.path.join(dir, "Rtree.libs") for dir in sitepackage_dirs + ], +) + # libsupermesh # example: # gcc -Ipath/to/libsupermesh/include @@ -205,7 +219,7 @@ def extensions(): name="firedrake.cython.supermeshimpl", language="c", sources=[os.path.join("firedrake", "cython", "supermeshimpl.pyx")], - **(mpi_ + petsc_ + numpy_ + libsupermesh_) + **(mpi_ + petsc_ + numpy_ + libsupermesh_ + spatialindex_) )) # pyop2/sparsity.pyx: petsc, numpy, cython_list.append(Extension( diff --git a/tests/firedrake/regression/test_real_space.py b/tests/firedrake/regression/test_real_space.py index 96839569bd..6b05ee73c5 100644 --- a/tests/firedrake/regression/test_real_space.py +++ b/tests/firedrake/regression/test_real_space.py @@ -1,4 +1,5 @@ import pytest +import pytest_mpi import numpy as np from firedrake import * @@ -428,3 +429,22 @@ def test_real_space_hex(): assert np.allclose(val.dat.data_ro, [2.]) val = assemble(inner(r, TestFunction(DG)) * dx) assert np.allclose(val.dat.data, [1., 1.]) + + +@pytest.mark.parallel +def test_real_space_repeated_assembly(): + """Test that repeated assembly of the same form works in parallel. + + This test is of particular importance to the Real space because Real + functions are globals and this can lead to problems with halo + increments. + + """ + mesh = UnitSquareMesh(3, 3) + R = FunctionSpace(mesh, "R", 0) + q = TestFunction(R) + + form = q*dx + q*ds # area + perimeter of the unit square = 1 + 4 = 5 + for i in range(3): + x = assemble(form) + pytest_mpi.parallel_assert(np.isclose(x.dat.data_ro.item(), 5))