Repository navigation
Conversation
|
This PR should reimplement |
a844911 to
f7f5ee3
Compare
f6d360f to
3537776
Compare
The DirichletBC setter called the Interpolator directly. It now keeps the Interpolate expression and assembles it with firedrake.assemble on every update, so the update is annotated for pyadjoint. The Interpolator is cached on the Interpolate expression, so it is built only once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dea0a5f to
bcb8a5f
Compare
Kernel calls with MIN_WRITE or MAX_WRITE access now pack a temporary that starts from the identity of the reduction, and unpack it with a "min" or "max" assignment. Assignments of type "min" and "max" now get their intents from assignment_type_as_intent, so they can be put in a loop. Fixes #5524 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
TSFC kernels add their result to the output argument. A MIN_WRITE or MAX_WRITE temporary that starts from the identity of the reduction therefore stays at the identity. op3.Function now takes requires_zeroed_output_arguments, like the PyOP2 Kernel did. When it is set, these temporaries start from zero. as_pyop3_local_kernel sets it for TSFC kernels. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ct/firedrake into pbrubeck/fix-minmax-write
…tsfc # Conflicts: # firedrake/interpolation.py # firedrake/tsfc_interface.py
quadrature_space is now a cached property. The adjoint weight, the reciprocal of the number of cells in the iteration set whose closure contains each node of the dual argument, is cached on the mesh, keyed by the dual space and the subset. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UGJuwu7dPzKbSmATxWo9Qc
#5127) * Symbolic coarsening of DirichletBC, and bc data interpolation from other spaces * BCBase.coefficients * Apply suggestion from @pbrubeck * Tape DirichletBC's internal interpolation for pyadjoint DirichletBC data on a different FunctionSpace was interpolated via a raw Interpolator.assemble() call, bypassing pyadjoint annotation. Route it through the top-level assemble() instead, stashing the Interpolate object so BaseFormAssembler reuses its cached Interpolator. DirichletBCBlock's adjoint also assumed the BC data lived on the same space as the BC itself; add the cross-space case, transforming the adjoint value through the adjoint of the (linear) interpolation operator. Add a regression test: a time-dependent heat equation with BC data interpolated from a different space at each step, verified with a Taylor test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * edit AGENTS.md * Tape DirichletBC's internal interpolation for pyadjoint DirichletBC data on a different FunctionSpace was interpolated via a raw Interpolator.assemble() call, bypassing pyadjoint annotation. Route it through the top-level assemble() instead, stashing the Interpolate object so BaseFormAssembler reuses its cached Interpolator. DirichletBCBlock's adjoint also assumed the BC data lived on the same space as the BC itself; add the cross-space case, transforming the adjoint value through the adjoint of the (linear) interpolation operator. Fix two further gaps found while making a reused-DirichletBC variant of the new test actually differentiable, rather than merely documenting it as broken: - DirichletBC.function_arg's setter now keeps the FloatingType's _ad_args in sync with the assigned value, so reusing a BC via set_value()/mutating its data re-tapes against the current value instead of the one captured at __init__. - NonlinearVariationalSolveBlock._forward_solve now applies its own recovered, per-step bcs to the cached forward_nlvs solver before solving, instead of solving with whatever the shared DirichletBC objects' live state currently happens to hold. Add a regression test, parametrized over constructing a fresh DirichletBC each timestep vs. reusing one via NonlinearVariationalSolver, both verified with a Taylor test. Document the underlying patterns in AGENTS.md in generalized form, so they transfer to future pyadjoint (or similar record/replay) bugs beyond this specific one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Apply suggestion from @pbrubeck * AGENTS.md: document the _ad_annotate_* mixin pattern Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * AGENTS.md: trim and reorder pyadjoint debugging lessons Sort from cheapest diagnostic wins to deepest architectural gotchas, and cut each lesson down to headline + one anchoring example. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * Compose DirichletBC adjoint with its taped boundary-value interpolation Make DirichletBCBlock depend on the Function the BC actually applies (bc._function_arg) rather than the raw user-supplied value. The interpolation (or, for elements without interpolation support, the projection) of the boundary value is re-taped through the annotated assemble/project when the BC is added as a dependency of another block, so its own AssembleBlock/ProjectBlock supplies the adjoint, TLM, and recompute. This deletes the hand-rolled interpolation pullback and the dead constant-handling machinery from DirichletBCBlock. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AGENTS.md: rewrite pyadjoint notes as composition-first design principles Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AGENTS.md: make pyadjoint principles generic Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Annotate Projector.project instead of rebuilding a Projector per update DirichletBC's projection fallback goes back to one cached Projector per boundary value: rebuilding a solver stack on every function_arg access crashes when the accessor fires inside PCSetUp_MG's coarsening callback (gmg on extruded hierarchies, where NCE/NCF cannot interpolate). The projection stays on the tape through a new ProjectorMixin that annotates ProjectorBase.project, taping one ProjectBlock per call against the current source value. Also locate the DirichletBCBlocks in test_bdy_control by type rather than by tape position, since the taped boundary-value interpolation now precedes them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AGENTS.md: branch pairing, CI triage, and PETSc-callback lessons Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * AGENTS.md: add design and debugging method Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * Add CLAUDE.md importing AGENTS.md Claude Code auto-loads CLAUDE.md from the repository root; the @AGENTS.md import inlines the agent instructions so they reach every session without relying on convention or memory. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * adjoint AGENTS.md * Apply suggestion from @pbrubeck * Apply suggestion from @pbrubeck * Drop unrelated AGENTS.md files * Remove unused firedrake.matrix import from bcs.py The import was left over from a merge resolution. firedrake.matrix imports DirichletBC from firedrake.bcs, so the import was circular and made `import firedrake` fail. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Clone DirichletBCs in the replay problem The problem that NonlinearVariationalSolver clones for replay copied the coefficients of F and J but kept the user's DirichletBCs, so a replay applied whatever boundary value the BC held at that time. The clone now reconstructs each DirichletBC with a copy of its boundary value, shared with F when the value is also a coefficient of F, and the checkpointed value is assigned before each replay. This replaces the reassignment of the cloned problem's bcs on every solve. Also shorten the comments on how DirichletBC is taped, and rename a local variable in EquationBCSplit.coefficients to match NonlinearVariationalSolver. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Probe the dual basis to choose interpolation or projection in DirichletBC The DirichletBC setter tried to interpolate the boundary value and fell back to projection on any NotImplementedError raised inside assemble. It now checks V.finat_element.dual_basis first, as in #5258, and builds either the interpolation or the Projector. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Add second-order Taylor test for time-dependent Dirichlet BC --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
|
|
||
| @cached_on( | ||
| get_obj=lambda self: get_mesh_topologies(self.ufl_interpolate), | ||
| get_key=lambda self: (self.dual_arg.function_space(), self.subset), |
There was a problem hiding this comment.
Should this be keyed on the source mesh too? In the submesh case
There was a problem hiding this comment.
It is cached on the source mesh. What does get_mesh_topologies return?
There was a problem hiding this comment.
Is it going to cache the weights on all meshes, including submeshes? If so, then if we have submesh S of a parent mesh M, an adjoint interpolation M -> M will store weights for the whole mesh on both M and S. Then if we do an adjoint interpolation (on the same dual space) S -> M we would use the weights for M.
There was a problem hiding this comment.
It's now cached on the source mesh
| @cached_property | ||
| def quadrature_space(self): |
There was a problem hiding this comment.
I suppose this is public API so should be deprecated correctly.
There was a problem hiding this comment.
It's a very young API, I don't think anyone has been messing with this.
There was a problem hiding this comment.
I can change it to a cached method if that makes people happier.
get_form_assembler already passes allocation_integral_types only to matrix assemblers, so the BaseForm visitor does not need to filter it. Nothing imports runtime_quadrature_element from pointeval_utils. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
df60e33 to
c407caf
Compare
An output that aliases an input is assembled into a temporary. The temporary must start as a copy of the output when the loop misses some target cells, otherwise their nodes are zeroed on copy-out. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A fused interpolation must reach every target cell. Instead of counting the cells of the interpolation loop on every rank, check the subset and the submesh hierarchy of the source and target meshes. This avoids an MPI reduction while the BaseForm DAG is traversed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
c407caf to
0e10a44
Compare
Co-authored-by: Leo Collins <leocollins511@gmail.com>
Co-authored-by: Leo Collins <leocollins511@gmail.com>
Summary
TSFC now compiles same-mesh
ufl.Interpolatenodes in normal Form kernels, and this PR ports the feature from PyOP2 to pyop3.InterpolateandForm(Interpolate)share one code generation path.MixedFunctionSpacesupportsmat_type="aij"natively.VertexOnlyMeshuses the same TSFC code generation.BaseFormAssembler.SameMeshInterpolatorno longer builds its ownop3.loops. It sends the interpolation through the form assembler:MINandMAXare reduced into the output asMIN_WRITEandMAX_WRITE. TSFC kernels add their result to the output argument, so pyop3 starts these temporaries from zero (see #5543).TwoFormAssembler._apply_bcnow takes the field label of a BC from the BC's own space. Before, a rectangular form such as an interpolation fromW = V * VintoVfailed.quadrature_spaceis now a cached property. The adjoint weight, the reciprocal of the number of cells in the iteration set whose closure contains each node, is cached on the mesh, keyed by the dual space and the subset.Stacked on #5543, which adds MIN/MAX reductions to pyop3 kernel calls.
Depends on firedrakeproject/fiat#294.
Fixes #5468
Evidence
mainwithout the fixes): 7 failures intests/firedrake/regression/test_interpolate.pytest_subset_adjoint_interpolation(new, 1 and 2 ranks) compares the matrix-free adjoint with the transposedaijmatrix, with and without a subset. It fails if the subset is left out of the cache key.Repeated interpolation on BDM2 16×16 through
Vq = V.quadrature_space, per call:Function(V).interpolate(q)Cofunction(Vq.dual()).interpolate(r)Merge Danger
Door: two-way
Blast Radius: interpolation
All same-mesh interpolation, and the Form kernels that contain it, now go through TSFC and the form assembler.
op3.Functiongets a new attribute, so kernel caches from before this change must be cleared withfiredrake-clean.V.quadrature_space()is now the propertyV.quadrature_space.The CI commits marked
DROP BEFORE MERGEinstall the FIAT branch and must be dropped before merging.Contribution assisted by Claude Code (Claude Opus 5.5).
🤖 Generated with Claude Code