Skip to content

Fix energy conservation in 1D VlasovAmpere runs with diagonal scaling - #741

Closed
max-models wants to merge 4 commits into
develfrom
fix-diagonal-scaling-1d
Closed

max-models wants to merge 4 commits into
develfrom
fix-diagonal-scaling-1d

Conversation

@max-models

Copy link
Copy Markdown
Member

Summary

Since #739 made diagonal_scaling=True the default of MassMatrixPreconditioner, the total energy of 1D VlasovAmpereOneSpecies runs (one element in e2 and e3) is no longer conserved: it drifts by ~1.5e-4 per step instead of ~1e-12. This breaks test_verif_VlasovAmpereOneSpecies.py::test_energy_conservation in #738. This PR fixes two bugs:

  1. VlasovAmpereCoupling: update the ghost regions of the mid-point field before the push. The pusher kernel reads the ghost regions of e_sum directly. They were only correct as long as the Schur solver happened to return valid ghost regions. With the diagonal scaling it doesn't, because its last step is a StencilDiagonalMatrix.dot, which writes only the owned entries. In 1D runs every evaluation reads ghost values. The other couplings (EfieldWeightsCoupling, CurrentCoupling6DCurrent, CurrentCoupling5DCurlb) already update the ghost regions before pushing.

  2. Diagonal scaling: include the band entries that wrap onto the diagonal. In a periodic direction with fewer points than the band width (2p + 1 > n, e.g. a single element), the band entries at offsets k = m n also lie on the main diagonal. StencilMatrix.diagonal() returns only the entry at k = 0, so the diagonal D of the mass matrix was wrong, and MassMatrixPreconditioner with diagonal scaling was no longer exact on the logical cube. Fixed with _stencil_local_diagonal in _local_diagonal, which MassMatrixPreconditioner._core_diagonal now uses.

Tests

  • New test_mass_matrices.py::test_mass_preconditioner_diagonal_scaling_periodic_exact: checks that the preconditioner with diagonal scaling inverts M0 to M3 exactly on the Cuboid, including grids with one element. Without the fix, 6 of the 12 cases fail.
  • Local runs, serial and on 2 ranks: test_preconditioner_transpose.py, test_stiffness_preconditioner.py, test_mass_matrices.py::test_mass_preconditioner* and test_poisson.py (127 / 151 passed). With Fix total energy at t=0 in Vlasov-Ampère/Maxwell models (ghost regions, control variate) #738 merged in, test_verif_VlasovAmpereOneSpecies.py passes (3 passed).

🤖 Generated with Claude Code

max-models and others added 2 commits October 10, 2026 08:18
In a periodic direction with fewer points than the band width (2p + 1 > n, e.g. a
single element in a 1D run), StencilMatrix.diagonal() returns only the entry at
offset 0, while the entries at offsets k = m n also lie on the main diagonal. The
diagonal D of the mass matrix was therefore wrong, and MassMatrixPreconditioner
with diagonal_scaling=True (the default since #739) was no longer exact on the
logical cube; in VlasovAmpereCoupling this broke energy conservation in 1D runs.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… before the push

The pusher kernel reads the ghost regions of e_sum directly. They were only correct
if the Schur solver happened to return its solution with valid ghost regions, which is
no longer the case with the diagonal scaling of MassMatrixPreconditioner (default since
#739): its last step is a StencilDiagonalMatrix.dot that writes only the owned entries.
In 1D runs (one element in e2, e3) every evaluation reads ghost values, and the total
energy was no longer conserved (drift ~1e-4 instead of ~1e-12). The other couplings
already update the ghost regions before pushing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@max-models
max-models requested a review from spossann October 13, 2026 12:48

@spossann spossann left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes here, got probably fixed in #738

@spossann spossann closed this Oct 13, 2026
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