Skip to content

Fix ZeroBaseForm holes in derivative expansion and action#494

Open
pbrubeck wants to merge 2 commits into
FEniCS:mainfrom
firedrakeproject:pbrubeck/zero-base-form-holes
Open

Fix ZeroBaseForm holes in derivative expansion and action#494
pbrubeck wants to merge 2 commits into
FEniCS:mainfrom
firedrakeproject:pbrubeck/zero-base-form-holes

Conversation

@pbrubeck

Copy link
Copy Markdown
Contributor

Summary

  • GateauxDerivativeRuleset had no rule for ZeroBaseForm itself, so differentiating an already-zero BaseForm (e.g. a Hessian of a Matrix) raised AssertionError: Rule not set for ZeroBaseForm in expand_derivatives.
  • map_integrands's FormSum branch collapsed to a bare 0 via sum([]) when every component vanished, silently dropping the argument shape; apply_derivatives had a matching fallback that rebuilt an argument-less Form([]) instead of a properly shaped ZeroBaseForm.
  • action() only expanded derivatives when the top-level object was already a Form/BaseFormOperator, so a CoefficientDerivative reachable only through a FormSum (e.g. from an affine residual F - b) survived unexpanded inside the resulting lazy Action. Widened the check to match adjoint()'s existing BaseForm handling.
  • ZeroBaseForm inherited BaseForm.empty(), which always returns False — gave it its own empty() returning True.

Test plan

  • pytest test/ (full suite, 969 passed)
  • New regression tests added to test/test_duals.py: nested differentiation of a Matrix (Hessian), a FormSum of Cofunctions whose components all vanish under differentiation, and action() on a FormSum no longer leaving a CoefficientDerivative behind.
  • ruff check

🤖 Generated with Claude Code

GateauxDerivativeRuleset had no rule for ZeroBaseForm itself, so
differentiating an already-zero BaseForm (e.g. a Hessian of a Matrix)
raised "Rule not set for ZeroBaseForm" in expand_derivatives.

map_integrands' FormSum branch collapsed to a bare `0` via `sum([])`
when every component vanished, silently dropping the argument shape;
apply_derivatives had a matching fallback that rebuilt an
argument-less Form([]) instead of a properly shaped ZeroBaseForm.

action() only expanded derivatives when the top-level object was
already a Form/BaseFormOperator, so a CoefficientDerivative reachable
only through a FormSum (e.g. from an affine residual F - b) survived
unexpanded inside the resulting lazy Action. Widened the check to
match adjoint()'s existing BaseForm handling.

Finally, ZeroBaseForm inherited BaseForm.empty(), which always
returns False -- give it its own empty() returning True.
@pbrubeck

pbrubeck commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

Note from Claude refering to https://github.com/firedrakeproject/firedrake/blob/49f19c307b1aa1cfa1386705f31ed72975e65c23/firedrake/formmanipulation.py#L92

I did not generalize this to map_integrands's plain-Form branch — I tried making it return ZeroBaseForm there too (which would have let formmanipulation.py's TODO disappear entirely), but it's provably wrong for transforms like ExtractSubBlock that replace arguments' function spaces rather than just adding a new one: a live regression (test_slate_infrastructure.py::test_blocks) showed it silently returning arguments on the wrong (unsplit) space. I reverted that piece. So the formmanipulation.py TODO and the identical pre-existing workaround in matrix_free/operators.py:150 stay as manual reconstructions — but now backed by an accurate comment explaining why expand_derivatives is genuinely required (the zero cancellation is a chain-rule consequence, not something detectable without differentiating), rather than the old "avoid expand_derivatives" aspiration.

Matches the existing # type: ignore on the neighboring Matrix rule,
whose signature is likewise incompatible with the Expr-typed base
process() the singledispatchmethod registry expects.
@pbrubeck pbrubeck mentioned this pull request Jul 11, 2026
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant