Skip to content

cpp/test/mesh/generation.cpp: build_tet cell-vertex order doesn't match hand-written expected tables #4522

Description

@jhale

Summary

cpp/test/mesh/generation.cpp has a CHECK_adjacency_list_equal test helper whose per-row structural comparison is commented out:

template <typename T>
void CHECK_adjacency_list_equal(
    const dolfinx::graph::AdjacencyList<T>& adj_list,
    const std::vector<std::vector<T>>& expected_list)
{
  REQUIRE(static_cast<std::size_t>(adj_list.num_nodes())
          == expected_list.size());

  // for (T i = 0; i < adj_list.num_nodes(); i++)
  // {
  //   CHECK_THAT(adj_list.links(i),
  //              Catch::Matchers::RangeEquals(expected_list[i]));
  // }
}

So every one of its ~9 call sites across the file (interval, triangle left/right/crossed diagonals, hexahedron, tetrahedron) currently only checks num_nodes() — the number of rows — never the actual vertex lists. The hand-written expected tables (e.g. the tet Kuhn-decomposition cell-to-vertex list in the "Box tetrahedron mesh" case) are dead text and give no structural coverage.

Reproduction

Re-enable the loop (with std::int32_t i instead of T i, which doesn't compile as written) and rebuild:

  for (std::int32_t i = 0; i < adj_list.num_nodes(); i++)
  {
    CHECK_THAT(adj_list.links(i),
               Catch::Matchers::RangeEquals(expected_list[i]));
  }

The "Box tetrahedron mesh" case then fails on the cell-to-vertex (connectivity(3, 0)) check, e.g.:

CHECK_THAT( adj_list.links(i), Catch::Matchers::RangeEquals(expected_list[i]) )
with expansion:
  { 0, 4, 1, 3 } elements are { 0, 1, 3, 4 }

CHECK_THAT( adj_list.links(i), Catch::Matchers::RangeEquals(expected_list[i]) )
with expansion:
  { 0, 5, 3, 2 } elements are { 0, 2, 5, 3 }

CHECK_THAT( adj_list.links(i), Catch::Matchers::RangeEquals(expected_list[i]) )
with expansion:
  { 0, 6, 4, 3 } elements are { 0, 4, 3, 6 }

CHECK_THAT( adj_list.links(i), Catch::Matchers::RangeEquals(expected_list[i]) )
with expansion:
  { 0, 7, 3, 5 } elements are { 0, 5, 7, 3 }

Each failure is a permutation of the same 4 vertices — never a wrong vertex set — for both float and double. The edge (connectivity(1,0)) and face (connectivity(2,0)) checks earlier in the same test case pass.

Why I didn't just fix it

Vertex order within a cell is geometrically meaningful (orientation), so this isn't obviously safe to paper over by rewriting the expected tables to match current output. But I also couldn't confirm it's a genuine build_tet bug, because:

  • Topology construction re-numbers/re-orders cell vertices during distribution (even for a single-rank MPI_COMM_SELF mesh in this test), and
  • no ordering contract is documented for connectivity(tdim, 0) anywhere I could find.

So this could be either:

  1. A real bug in build_tet's Kuhn decomposition (cpp/dolfinx/mesh/generation.h), or
  2. Expected-value tables that were written against raw build_tet output and never updated after a later, legitimate reordering elsewhere in the topology-construction pipeline, or
  3. Working as intended, with the row order simply never having been a documented guarantee — in which case the test itself is asserting something that shouldn't be asserted, and should compare vertex sets per row (or an orientation-aware equality) rather than exact sequences.

Where this came from

Found while working through the cpp/dolfinx/mesh/ code review test-gap appendix (re-enabling this exact check was one of its proposed items, F13-9). Verified against commit 44891b763603cf2c6b038597b031f2cb29155ce1 on mainbuild_tet and this test file are unmodified relative to that commit, so the repro applies directly to main.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions