Skip to content

Firedrake meeting 2026 06 16

Jump to bottom
India Marsden edited this page Jun 16, 2026 · 24 revisions

Date and time 2026-06-16 1600 UTC+1

Action Items

  1. Pick Chair and Minuter (LC to pick)
  2. ALL: (ongoing) triage the open issues and confirm if they are indeed still open (and perhaps provide labels)
  3. ALL: do things with SV's branches
  4. DH: Email to Andreas to have 2 (+ others!!!) loopy PRs merged TODO: FIND OUT WHICH PRS THESE ARE
  5. DH: Talk to GregVernon about PR#2116.
  6. JHC: New manual release

Agenda

Present: DH, CW, DG, LC, IM, PB, JHC (from halfway!) AT, AB (Corintis)

Apologies: JHC

LC/DH: Need someone to cover on 29th/30th June

We have a summer student starting and we're both away for these days - can CW/JHC meet and help her get set up with Firedrake.

IM to handle this.

SG: PRs

pyadjoint #248 + firedrake #5168: fixes the SingleMemoryStorageSchedule clearing bugs from #211 and #260 by keeping checkpoints until a reverse pass revises the adjoint dependencies. The red CI isn't from this PR; it's the pre-existing #257 breakage where the is_control guard stops controls being cleared, so the firedrake test helpers need to skip controls (folding into #5168).

Minutes:

  • Presentation of issue: Firedrake eagerly sets up MPI
  • DH: This issue come from PETSC. When you import Firedrake we import petsc4py which takes control of the MPI global import
  • (cont) Therefore this issue should be taken to petsc4py in the first instance. It is interesting that mpi4py handles this differently, as they have the same author.
  • Also, there are lazy import solutions that could work.
  • Q: Does BLAS also get effected when import?
  • Yes, we try to set number of threads programmatically. This could be fixed/evaded without too much trouble.
  • Q: Could this be moved to an init function?
  • In theory yes, but this is a big breaking change that would have to go through our deprecation cycle if approved.
  • DH: Top Short term option - Use separate lazy import package to import firedrake. Second as a potential is a separate python package "lazy_firedrake" that ensure we don't modify the global state on import.
  • Long term: Joint conversation to be started between Corintis, Firedrake and PETSc about the issues in petsc4py. Issue to be created by Corintis team.

Discussion Points from SiaG: Adjoint taping issues, appctx and projection

  • Stale appctx during adjoint replay. When an annotated NLVS carries an appctx, the replay machinery deep-copies the form coefficients but leaves the appctx pointing at the originals, so a preconditioner like MassInvPC sees end-of-run values on replay. Real case is a fieldsplit Stokes Schur complement where the viscosity is orders of magnitude off and the pressure solve diverges. There are two PRs because the code exists in two forms: #5171 fixes the legacy cloning path on release and is the actual bug fix, #5170 reinstates the same fix against Josh's nlvs-hessian-fix (#4638) which retires the methods #5171 patches, and also covers the TLM path. Connor's alternative on #5171 is to teach the linear solver path to accept appctx so the pop and the default_appctx fallback become unnecessary. Want to settle: minimal contained fix into release now versus holding for the linear-solver change, and how #5170 folds into #4638. And generally for Josh/Angus/... to be aware of this issue.

Minutes:

  • Pull request 5171 is the one to go in to release- comments added. Make sure it errors when users try to pass appctx using both routes.

  • Need to deprecate passing an appctx in solver options to ensure there is only one way to pass the appctx. Put this in main such that it is deprecated during next release cycle.

  • Projection annotation. Several distinct faults. Solver parameters passed to project were never recorded on the block, so replay and adjoint silently used the global defaults (#5172, fixed by #5175). Direct Projector(...).project() is not taped at all (#5176). There are two forward paths (same-space assigns, cross-space solves) and three entry points. #5175 is a contained record-the-parameters fix that works but cannot capture slate inverse or constant_jacobian and does nothing for #5176. Question is whether to redesign around a block that holds the Projector (like SupermeshProjectBlock), which reshapes the same blocks/solving.py Josh's #4638 rewrites, so it wants building on top of that refactor.

Minutes:

  • The correct choice for Projector is to tape the class that it delegates the method to - ensure the execution point is taped. JHC to manage.

PB: Change default model for DirichletBC

https://github.com/firedrakeproject/firedrake/issues/5177

The current default (restrict=False, pre_apply_bcs=True) poses the problem on the full space, and the BCs are directly imposed on the initial guess so there's no need to update them. However, the method is flawed:

a. Assumes that SNES updates satisfy homogeneous BCs, but sometimes the PC might couple BCs with the interior (e.g. GAMG does not solve the identity block on the BC rows exactly), and this spoils the BC values, without the possiblity of correcting them.

b. It can silently lead to bad initial guesses with sharp gradients that cause NaN in the first residual. (E.g. for Navier-Stokes on a lid-driven cavity, an initial guess u=0 will not be respected, hence the initial Jacobian is not Stokes.)

The proposal to set pre_apply_bcs=restrict=False as default, which does not suffer from either a and b. If we want to be able to restrict, the other method that we should keep is pre_apply_bcs=restrict=True, which only suffers from b, as b is inherent to restrict=True (we cannot set restrict=True with pre_apply_bcs=False because restrict is unable to update the BC values).

Minutes:

  • This is a potentially breaking API change - must be put through deprecation pathway.

  • This boils down to restricting the options to (True, True) and (False, False) and removing the options (True, False), (False, True).

  • It can be solved by removing pre_apply_bcs and using only restrict - this impacts where pre_apply_bcs is currently + breaks api

  • Another option is to keep pre_apply_bcs and fix (a) by correcting the BCs each time.

  • Bigger picture solution: Provide a pre_apply_bcs preconditioner that handles this. Same theoretically could be done for restrict.

  • Summary: In Main, warn if users don't set pre_apply_bcs to False, noting it will be removed. This goes into release in October. Then, after the release, main can be changed so that we cannot have restrict=False and pre_apply_bcs set to anything other than False.

PB: Interpolation into EnrichedElement

We should let Gusto people know, as they have been unable to do interpolation into RTCF elements.

Deferred.

DG: Review changes to ReducedFunctional and discuss integrating TAO solver with EnsembleReducedFunctional

Right now tests are only on the R space. Solution on Slack - you have to pass the right comm to the tao solver.

Merge PRs

Date of next meeting

1600 UTC+0100 2026-06-23

Home

Installation

Firedrake governance

Other pages

Clone this wiki locally