Skip to content

Raise a DiscretisationError for FiniteVolume2D meshes with one node in a direction - #5838

Open
aabills wants to merge 2 commits into
mainfrom
fix/fv2d-single-tb-node
Open

aabills wants to merge 2 commits into
mainfrom
fix/fv2d-single-tb-node

Conversation

@aabills

@aabills aabills commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Description

pybamm.lithium_ion.BasicDFN2D() solved with z_2d set to 1 point failed deep in FiniteVolume2D.harmonic_mean with an opaque scipy ValueError: axis 1 index 139 exceeds matrix dimension 70.

A single node in either direction can't be supported by the 2D finite volume method as written. The node-to-edge shifts (harmonic and arithmetic mean) and the boundary extrapolation all estimate the exterior edges by linear extrapolation from the first or last two nodes. With one node there are no interior edges and no second node to extrapolate from. Supporting it would need a constant-extrapolation special case in several operators, and a 2D model with one node in a direction doesn't vary in that direction, so it is the 1D model.

FiniteVolume2D.build now checks every SubMesh2D and raises a pybamm.DiscretisationError when either direction has fewer than 2 nodes. The error names the direction and domain and points to refining the mesh or using a 1D model. Ghost-cell submeshes are one node thick by construction and are skipped. Unstructured submeshes, which report npts_tb = 1, are not SubMesh2D and are unaffected. Meshes with at least 2 nodes in both directions behave exactly as before.

Meshes with exactly 2 nodes in a direction still hit an IndexError in boundary_value_or_flux. That case is handled separately and is not touched here.

Type of change

  • Bug fix. CHANGELOG entry under ## Bug fixes.

Important checks:

  • No style issues: pre-commit run on the changed files
  • Tests pass: full unit suite (4708 passed; the only failures are the 3 GIF-writer tests that fail on the local machine for lack of a working movie writer) and the 2D integration tests (12 passed)
  • The documentation builds: not run, no docs changes
  • Code is commented for hard-to-understand areas
  • Tests added: test_single_node_direction_raises covers 1 node in tb and in lr

🤖 Generated with Claude Code

Node-to-edge shifts and boundary extrapolation in FiniteVolume2D need at
least two nodes per direction, so a 2D submesh with one node (e.g.
BasicDFN2D with z_2d = 1) failed deep in harmonic_mean with an opaque
scipy ValueError. build() now raises a DiscretisationError up front;
ghost-cell submeshes are exempt.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@aabills
aabills requested a review from a team as a code owner October 2, 2026 22:40
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

🐰 Bencher Report

ProjectPyBaMM
Branchfix/fv2d-single-tb-node
Testbedbare-metal

⚠️ WARNING: Truncated view!

The full continuous benchmarking report exceeds the maximum length allowed on this platform.

🐰 View full continuous benchmarking report in Bencher

@codecov

codecov Bot commented Oct 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.09%. Comparing base (445ba33) to head (324a390).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5838   +/-   ##
=======================================
  Coverage   98.09%   98.09%           
=======================================
  Files         346      346           
  Lines       35354    35361    +7     
=======================================
+ Hits        34680    34687    +7     
  Misses        674      674           

☔ 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.

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.

1 participant