Skip to content

Add unstructured mesh infrastructure - #5687

Open
aabills wants to merge 6 commits into
mainfrom
ufv-1-meshing
Open

Add unstructured mesh infrastructure#5687
aabills wants to merge 6 commits into
mainfrom
ufv-1-meshing

Conversation

@aabills

@aabills aabills commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add UnstructuredSubMesh, generators (UnstructuredMeshGenerator, UserSuppliedUnstructuredMesh, TaggedSubMeshGenerator), and Mesh interface coupling.
  • Independent of the VectorField PR; both land before the spatial-method PR.

Stack

  1. Generalise VectorField to N components #5686 — VectorField N-comp
  2. This PR (Add unstructured mesh infrastructure #5687) — Meshing
  3. Add unstructured finite volume spatial method #5688 — Spatial method + ProcessedVariable
  4. Add VTK plotting for unstructured meshes #5689 — VTK plotting
  5. Add unstructured 2D/3D DFN battery models #5690 — Unstructured DFN models

Full pre-split branch preserved on #5397 / backup/unstructured-finite-volume-full.

Test plan

  • tests/unit/test_meshes/test_unstructured_submesh.py (41 tests)
  • CI unit suite

Also in this stack: #5691 deprecates pybamm.Magnitude.

Introduce UnstructuredSubMesh, mesh generators, and Mesh interface
coupling so arbitrary 2D/3D domains can be discretised later. Extracted
from the unstructured finite-volume stack for isolated review.

Co-authored-by: Cursor <cursoragent@cursor.com>
@codecov

codecov Bot commented Jul 31, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.24561% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 98.21%. Comparing base (1e9fbce) to head (c3479ab).

Files with missing lines Patch % Lines
...s/pybamm/src/pybamm/meshes/unstructured_submesh.py 98.19% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5687      +/-   ##
==========================================
+ Coverage   98.12%   98.21%   +0.08%     
==========================================
  Files         339      340       +1     
  Lines       31996    32622     +626     
==========================================
+ Hits        31396    32039     +643     
+ Misses        600      583      -17     

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

aabills and others added 3 commits July 31, 2026 15:08
`_combine_unstructured_submeshes` put 110 lines of unstructured-specific
node welding in the generic `meshes.py`, which then needed two
function-level `from .unstructured_submesh import ...` calls to dodge the
module-level cycle (`unstructured_submesh` already imports `SubMesh` and
`MeshGenerator` from `meshes`).

Move it to `UnstructuredSubMesh.combine`, where `_hex_to_tet` and the
class itself are local, and reach `compute_interface_data` through the
`pybamm` namespace like the rest of `meshes.py` does. `meshes.py` no
longer references `unstructured_submesh` at all.

Pure move: the welding and boundary-tag-recovery bodies are unchanged
apart from `pybamm.UnstructuredSubMesh(...)` becoming `cls(...)`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…_ordering

`interface_data` stores "left_cells" as this mesh's cells and "right_cells"
as the neighbour's, but `optimize_ordering` permuted both with this mesh's
`inv_perm`. That silently re-pointed the neighbour's indices, and where the
neighbour has fewer cells it produced out-of-range indices (an 18-cell mesh
next to a 12-cell one yielded `right_cells.max() == 17`).

`compute_interface_data` also gives the neighbour a mirrored view of the
same pairing, which aliases the very array being replaced, so the mirror
went stale. Permute only this mesh's indices and write the result through
to the neighbour's mirror.

The existing test asserted only on "left_cells" — the one key that was
already correct — so add a case that pins the neighbour's indices and the
mirror, using meshes of different sizes so a mis-permutation is caught by
bounds rather than by luck.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`UnstructuredSubMesh.combine` welds coincident interface nodes so the seam
becomes internal faces and TPFA carries flux across it. When the interface
nodes don't coincide — mismatched transverse grids, wrong coordinate units,
or a non-conforming input mesh — nothing welds, the seam stays boundary
faces on both sides, and the domains form separate connected components. The
solve then runs to completion with the regions silently decoupled and
returns a physically wrong answer.

Add a post-weld check that the combined mesh is a single connected
component, using the integer face_owner/face_neighbor connectivity so it
introduces no distance tolerance. Under the conforming-interface
requirement welding is all-or-nothing, so total disconnection is the only
failure mode and this check is sufficient. Document the conforming
requirement on the two gmsh-reading generators.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
aabills and others added 2 commits July 31, 2026 16:09
Four review findings on #5687:

- Reject non-manifold input: a face shared by more than two cells matched
  neither the internal (count 2) nor boundary (count 1) branch of
  _build_face_connectivity and silently vanished from the face list,
  losing flux paths. Now raises GeometryError.

- Make the 3D generator honour element_type: _generate_3d always built
  hexes regardless of the argument (docstring claimed a tetrahedron
  default). Hexahedron stays the default, matching existing tests and the
  3D DFN model; "tetrahedron" now works and records _hex_gen_params so
  combine() can rebuild tet domains with a cumulative parity offset,
  keeping interface triangulations conforming — previously that combine
  branch was unreachable. Unknown types raise.

- Remove dead code: _cell_faces (unused since the vectorised
  connectivity path) and an n_fpc assignment overwritten two lines later.

- Conventions: framework errors now raise pybamm.GeometryError instead
  of ValueError/KeyError/RuntimeError; meshio is imported via
  pybamm.import_optional_dependency; the CHANGELOG bullet carries the PR
  link.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov flagged the meshio-backed generators as entirely untested. Add
tests for UserSuppliedUnstructuredMesh (whole-file load with 2D trimming,
subdomain filtering, missing-cell-data and unsupported-cell-type errors,
lims-to-domain mapping) and TaggedSubMeshGenerator (region extraction with
coordinate scaling, missing region, region with no tets), using synthetic
meshio objects injected into the class cache so no .msh round-trip is
needed. Also cover combine()'s custom boundary-tag recovery,
compute_interface_data's no-matching-faces and transverse-mismatch raises,
and Mesh leaving interface_data empty when grids don't pair.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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