Skip to content

Fix double-counted ghost contributions in parallel Hessian action - #100

Open
finsberg wants to merge 2 commits into
mainfrom
fix/hessian-cross-term-ghost-double-count
Open

finsberg wants to merge 2 commits into
mainfrom
fix/hessian-cross-term-ghost-double-count

Conversation

@finsberg

Copy link
Copy Markdown
Member

_ProblemBlockBase.evaluate_hessian_component accumulated the pure d2F/dm2 term and each mixed d2F/(dm dc2) term into the same output vector with repeated assemble_compiled_form calls. Each call ends in scatter_reverse(add) + scatter_forward, which leaves the ghost entries holding copies of their owners' values, so the next call's reverse scatter added those copies back into the owners again: every shared dof of the first term was counted twice.

Serial runs have no ghosts and were unaffected; in parallel any solve with a non-zero mixed term (e.g. a control multiplying the previous state in a time-stepping right-hand side) got a wrong Hessian action, visible as a Hessian Taylor rate of 2 instead of 3.

Add assemble_compiled_forms, which sums every form's local contribution and exchanges ghosts once, and use it there. The new regression test fails under CI's mpirun -n 2 run without the fix.

_ProblemBlockBase.evaluate_hessian_component accumulated the pure
d2F/dm2 term and each mixed d2F/(dm dc2) term into the same output
vector with repeated assemble_compiled_form calls. Each call ends in
scatter_reverse(add) + scatter_forward, which leaves the ghost entries
holding copies of their owners' values, so the next call's reverse
scatter added those copies back into the owners again: every shared dof
of the first term was counted twice.

Serial runs have no ghosts and were unaffected; in parallel any solve
with a non-zero mixed term (e.g. a control multiplying the previous
state in a time-stepping right-hand side) got a wrong Hessian action,
visible as a Hessian Taylor rate of 2 instead of 3.

Add assemble_compiled_forms, which sums every form's local contribution
and exchanges ghosts once, and use it there. The new regression test
fails under CI's `mpirun -n 2` run without the fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@finsberg
finsberg requested a review from jorgensd September 30, 2026 13:53
@jorgensd

jorgensd commented Oct 2, 2026

Copy link
Copy Markdown
Member

This branch has not been deployed

No deployments
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.

2 participants