Add AGENTS.md and CLAUDE.md#495
Open
pbrubeck wants to merge 4 commits into
Open
Conversation
AGENTS.md documents UFL's Expr/BaseForm primal-dual split, the singledispatchmethod-based DAGTraverser dispatch pattern used throughout apply_derivatives.py, map_integrands' recursive form transform, and the PR/lint workflow (fork push + origin-targeted PR, the three separate lint.yml stages). The Anti-Patterns section is built directly from the ZeroBaseForm/argument-preservation bugs fixed in the companion PR, including a documented dead end (naively generalizing map_integrands' Form branch the way its FormSum branch already works) so a future agent does not have to rediscover it via a test regression. CLAUDE.md imports AGENTS.md so Claude Code picks it up automatically, mirroring the pattern already in use downstream in Firedrake.
Generalizes lessons from today's two PRs: JacobianCanceller's design was adapted from GEM's own Delta-cancellation passes rather than derived from scratch, and the map_integrands Form/FormSum asymmetry cost a real regression before being scoped correctly. Encourages borrowing proven designs from sibling/downstream packages, matching new dispatch rules to the nearest *structurally* similar existing one (boilerplate included), classifying which structural category a fix belongs to before generalizing it to a sibling code path, and isolating cross-package regressions with a one-delta reproduction.
Tightened every section for scannability (shorter bullets, less repeated prose, dedup the "how to add a rule" guidance that was split across Project Architecture and Pattern Matching). Moved Pattern Matching For Planning And Debugging to just before Anti-Patterns. Added lessons from the CoefficientDerivativeRuleDispatcher fix: - Debugging: DAGTraverser tracebacks are mostly KeyError cache-miss noise (skip to the last exception, read `self`'s class at each frame); diff a narrow code path against the full pipeline on the same input to localize where they structurally diverge. - Pattern Matching: a fix that trades one failing test for another is a context signal (type-keyed dispatch can't distinguish "inside a region" from "outside it"), not a cue to add more special cases. - Anti-Patterns: a new worked example (narrowing a dispatcher's node-type coverage without narrowing its point of use) built directly from the Grad-inside-CoefficientDerivative bug.
Structural/.signature() agreement between two independently-built expressions, or the absence of an exception, does not prove a derivative or simplification is correct -- both paths can share the same wrong rule. Require verifying the actual mathematical claim (numeric evaluation, assemble-and-compare, or a Taylor test for derivatives), and reserve structural comparisons for checking that two constructions agree with each other, not that either is right.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
AGENTS.md, documenting UFL's architecture (theExpr/BaseFormprimal-dual split, thesingledispatchmethod-basedDAGTraverserdispatch pattern inapply_derivatives.py,map_integrands's recursive form transform), coding conventions (Google-style docstrings, the# type: ignoreconvention on AD rule registrations, ruff's 100-char line length), testing conventions, and the fork-push/origin-PR + three-stage-lint workflow this repo uses.0/Form([])instead ofZeroBaseFormfor a form that turned out to be zero (losing argument shape) and a documented dead end from that same investigation (Fix ZeroBaseForm holes in derivative expansion and action #494 — naively generalizingmap_integrands'sFormbranch the way itsFormSumbranch already works), plus a third worked example from fixingpbrubeck/fix/replace-coefficient-derivative'sCoefficientDerivativeRuleDispatcher: narrowing a dispatcher's node-type coverage (to protect foreignDerivativesubtypes) without narrowing where that narrowing applies, which silently also left spatialGradun-normalized inside aCoefficientDerivative's own content.CoefficientDerivativeRuleDispatcherfix: search sibling/downstream packages for prior art before designing a new algorithm (Cancel contractions of the Jacobian with its inverse #496'sJacobianCanceller/IdentityEliminatordeliberately ports GEM's own Delta-cancellation passes), match a new dispatch rule to the nearest structurally similar existing one, classify which structural category a fix belongs to before generalizing it to a sibling code path, isolate cross-package regressions with a one-delta reproduction, and — new — treat a fix that trades one failing test for another as a context signal (type-keyed dispatch can't distinguish "inside a region" from "outside it") rather than a cue to add more special cases.DAGTraversertracebacks are mostlyKeyErrorcache-miss noise (skip to the last exception, readself's class at each frame to see which dispatcher/ruleset was active), and diffing a narrow code path against the full pipeline on the same input localizes exactly which node differs.CLAUDE.mdimportingAGENTS.mdso Claude Code picks it up automatically, mirroring the pattern already in use downstream in Firedrake.Test plan
🤖 Generated with Claude Code