fix: keep symbolic mesh scaling out of finite-volume matrices by BradyPlanden · Pull Request #5781 · pybamm-team/PyBaMM

BradyPlanden · 2026-09-23T18:47:58Z

Description

A mesh with a symbolic length (SymbolicUniform1DSubMesh, used when a length such as a particle radius is an InputParameter) lets a model be discretised once and solved for many geometries. On main, FiniteVolume folds that input-dependent scaling into its matrices, building kron(eye(R), stencil * scale) or (Matrix * scale) @ y. Only CasADi can evaluate those trees. KroneckerProduct has no Jacobian, MatrixMultiplication's Jacobian needs a constant left operand, and the Python/JAX code generator writes the literal text kronecker product into the generated code, which is a SyntaxError. So an SPM or DFN with an input particle radius fails with convert_to_format="python" or "jax", and in symbol.jac. Separately, an integral over a symbolic secondary dimension raised a ShapeError because it used the primary domain's edges.

This PR keeps every matrix constant on a symbolic mesh and applies the scaling to the operand or the result instead. Numeric meshes are untouched: across 15 model configurations (SPM, SPMe, DFN with 1D/2D current collectors, thermal, SEI and size distributions, MPM, MSMR, lead-acid, spectral volume) the discretised trees have identical node sequences and bit-identical constants.

Vectors are repeated over auxiliary domains with a constant tiling matrix rather than a Kronecker product.
The public matrix builders (gradient_matrix, divergence_matrix, definite_integral_matrix, the indefinite-integral matrices) return a constant stencil times a row or column scaling, which folds back into a single Matrix on a numeric mesh. The operators apply it through _apply_matrix, as scale * (stencil @ y) or stencil @ (scale * y), so a subclass that overrides a public builder is still honoured.
Boundary values and gradients, harmonic means and internal Neumann conditions scale or divide the product rather than the matrix, and the secondary-dimension integral uses its own domain's edges.
Tests compare symbolic and numeric meshes operator by operator (values, the Python evaluator, the Jacobian, and that every matrix factor is constant), plus an SPM and a DFN with input particle radii whose IDAKLU solutions match the numeric mesh.

This is one of the backend-neutral pre-flight PRs split out of the Rust execution engine (#5732). It has no Rust dependency.

Type of change

Add an entry under # [Unreleased] in CHANGELOG.md, in one of ## Breaking changes, ## Deprecated, ## Features, or ## Bug fixes (include PR number; breaking and deprecation entries need a one-line migration note). Internal-only PRs — refactor, docs, CI, tests — can skip this. See RELEASE.md for the full policy.

Important checks:

Please confirm the following before marking the PR as ready for review:

No style issues: nox -s pre-commit
All tests pass: nox -s tests
The documentation builds: nox -s doctests
Code is commented for hard-to-understand areas
Tests added that prove fix is effective or that feature works

codecov · 2026-09-23T19:06:07Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.99%. Comparing base (c8b9f80) to head (b977002).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5781      +/-   ##
==========================================
- Coverage   98.00%   97.99%   -0.01%     
==========================================
  Files         346      346              
  Lines       35300    35309       +9     
==========================================
+ Hits        34595    34601       +6     
- Misses        705      708       +3

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

martinjrobins

Thanks @BradyPlanden, the kron(...) pattern is repeated quite a few times where you are repeating either a vector or matrix, might be worth having a utility function for this (or actually updating pybamm.kronecker_product to detect the specific patterns and apply restructuring appropriately

martinjrobins · 2026-09-25T08:43:05Z

        # issue
-        matrix = pybamm.kronecker_product(
-            pybamm.Matrix(eye(second_dim_repeats, dtype=np.float64)), sub_matrix
+        stencil = pybamm.Matrix(


there is that same pattern again

Yeah, agreed. I've add a helper now (_block_diagonal) and refactored all the call sites for this style concatenation.

martinjrobins

happy with this, thanks @BradyPlanden

On a mesh with a symbolic length, build every finite-volume matrix from constants and scale its operand or result, so the Python/JAX evaluators and pybamm's Jacobian handle the model; numeric-mesh trees are unchanged.

comment-slop judges a whole docstring once any line changes, and main's Notes derivation is over its length limit. _scaled_matrix documents the Matrix-or-product return.

… helper Add SpatialMethod._block_diagonal for the kron(eye(repeats), block) pattern and use it for the gradient, divergence and integral stencils, as suggested in review. Matrices are unchanged; a test checks each operator matrix on a mesh with a secondary domain is the block diagonal of its single-point one.

Replace the remaining kron(eye(repeats), block) constructions in FiniteVolume, SpectralVolume and SpatialMethod with _block_diagonal, and drop the row-slicing comment copied at each site. The delta function no longer calls pybamm.kronecker_product. Matrices are unchanged. Add tests with a secondary domain for the internal Neumann condition, EvaluateAt and the node/edge shifts, which had only been exercised with a single repeat.

…olumeUnstructured Replace every kron(eye(repeats), block) in the two methods with _block_diagonal and drop the copied row-slicing comment. The kron(ones((repeats, 1)), ...) tiling calls are a different pattern and stay. Matrices are unchanged. Add tests with a secondary domain: FiniteVolume2D gradient components, boundary values and gradients, integrals, shifts, internal Neumann condition and concatenation had only run with a single repeat, as had the unstructured non-orthogonal Laplacian, divergence, boundary and corner values and both internal Neumann paths.

github-actions · 2026-09-30T08:22:00Z

Bencher Report

Project	PyBaMM
Branch	a1/fv-symbolic-mesh-scaling
Testbed	bare-metal

⚠️ WARNING: Truncated view!
The full continuous benchmarking report exceeds the maximum length allowed on this platform.

🚨 1 Alert

🐰 View full continuous benchmarking report in Bencher

Move the Unreleased entries under v26.10.0.0 and bump CITATION.cff. Move the #5781, #5784, #5813 and #5817 entries out of v26.9.0.0, which was tagged before they merged, and add the missing pybammsolvers>=0.11.0 breaking change. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

BradyPlanden requested a review from a team as a code owner September 23, 2026 18:47

BradyPlanden force-pushed the a1/fv-symbolic-mesh-scaling branch from 9a9678c to 42f3626 Compare September 23, 2026 18:48

martinjrobins requested changes Sep 25, 2026

View reviewed changes

BradyPlanden force-pushed the a1/fv-symbolic-mesh-scaling branch from 22d3e29 to 5d7f265 Compare September 30, 2026 08:04

BradyPlanden requested a review from martinjrobins September 30, 2026 08:08

martinjrobins approved these changes Sep 30, 2026

View reviewed changes

BradyPlanden added 5 commits September 30, 2026 09:15

fix: keep symbolic mesh scaling out of finite-volume matrices

52ff4dd

On a mesh with a symbolic length, build every finite-volume matrix from constants and scale its operand or result, so the Python/JAX evaluators and pybamm's Jacobian handle the model; numeric-mesh trees are unchanged.

style: leave the indefinite-integral docstring as on main

6cc2ffc

comment-slop judges a whole docstring once any line changes, and main's Notes derivation is over its length limit. _scaled_matrix documents the Matrix-or-product return.

BradyPlanden force-pushed the a1/fv-symbolic-mesh-scaling branch from 5d7f265 to b977002 Compare September 30, 2026 08:15

BradyPlanden enabled auto-merge (squash) September 30, 2026 08:15

BradyPlanden merged commit 07d642c into main Sep 30, 2026
67 of 70 checks passed

BradyPlanden deleted the a1/fv-symbolic-mesh-scaling branch September 30, 2026 08:59

rtimms mentioned this pull request Oct 9, 2026

Release v26.10.0.0 #5883

Merged

Navigation Menu

Repository navigation

fix: keep symbolic mesh scaling out of finite-volume matrices - #5781

fix: keep symbolic mesh scaling out of finite-volume matrices#5781
BradyPlanden merged 5 commits into
mainfrom
a1/fv-symbolic-mesh-scaling

BradyPlanden commented Sep 23, 2026

Uh oh!

codecov Bot commented Sep 23, 2026 •

edited

Loading

Uh oh!

martinjrobins left a comment

Uh oh!

Uh oh!

Uh oh!

martinjrobins Sep 25, 2026

Uh oh!

BradyPlanden Sep 30, 2026

Uh oh!

martinjrobins left a comment

Uh oh!

github-actions Bot commented Sep 30, 2026 •

edited

Loading

⚠️ WARNING: Truncated view!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

BradyPlanden commented Sep 23, 2026

Description

Type of change

Important checks:

Uh oh!

codecov Bot commented Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

martinjrobins left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

martinjrobins Sep 25, 2026

Choose a reason for hiding this comment

Uh oh!

BradyPlanden Sep 30, 2026

Choose a reason for hiding this comment

Uh oh!

martinjrobins left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Sep 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bencher Report

⚠️ WARNING: Truncated view!

🚨 1 Alert

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov Bot commented Sep 23, 2026 •

edited

Loading

github-actions Bot commented Sep 30, 2026 •

edited

Loading