Symmetric-tensor and deferred writes through .array were silently lost

[lmoresi]

Two silent defects on the write path the style charter makes mandatory for new code (§7: "new code uses the array property"). Both predate this branch, both are reachable from the interface docs/developer/subsystems/data-access.md recommends, and both lose data without raising.

1. A symmetric tensor's off-diagonal write vanished

(i, j) and (j, i) share one stored column, and the pack-back loop visits every (i, j), so the pair's second visit overwrote the first:

var.array[:, 0, 1] = 30.0    # -> stored 0.0     silently discarded
var.array[:, 1, 0] = 30.0    # -> stored 30.0    worked

Lower beat upper purely because it came last in the loop. In 3-D all three upper off-diagonals vanished. Reading .array back showed the write had not happened — so a stress or strain-rate history assembled component by component through the documented interface lost every shear term.

The half that changed is now mirrored onto the half that did not. Setting the two corners to different values in one assignment is refused, rather than silently resolved by loop order.

The swarm variant failed loudly instead of silently — its pack was a flat reshape that ignored symmetric storage, so (N, 2, 2) became (N, 4) and the assignment could not broadcast into (N, 3). A symmetric tensor on a swarm could not be written through .array at all.

2. Deferred writes on a swarm kept only the last

The swarm view read its current values straight from the PETSc field. synchronised_array_update() defers the pack, so inside that context the field still held the pre-context values and every write started from them:

with uw.synchronised_array_update():
    velocity.array[:, 0, 0] = 1.0
    velocity.array[:, 0, 1] = 2.0
# -> [0., 2.]    the first write was overwritten

Mesh variables were immune, because their view reads and writes the canonical array. The swarm view now does too.

Why the suite missed both

It was not thin. 219 of 399 test files touch .array, and 167 writes use the three-index form. But:

• 148 of those are [:, N, N] with literal integers. Across the whole suite there are exactly two off-diagonal writes, and both pick [:, 1, 0] — the corner that survived. [:, 0, 1], the natural way to write a shear component, appears nowhere.
• The one file that writes all four corners, test_0102_meshvariable_stats.py, is on a VarType.TENSOR, which stores them independently and cannot expose it. Its comment says "Non-symmetric, spatially varying" — the author picked the immune case.
• Eight blocks write one variable several times inside a synchronised context. Every one is on a mesh variable.

Both defects sat in the single cell of the shape × carrier × index matrix nobody had filled. The new tests are parametrised over that product rather than written one case at a time — test_a_symmetric_off_diagonal_write_lands_on_every_carrier is 6 cases from 2 lines, 3 of which were failing.

Each fix was verified by reverting it alone: exactly its own tests fail, no others.

Tests

tests/test_0509_symmetric_tensor_array_writes.py, 19 tests. Full level_1 and tier_a: 1187 passed, 3 skipped, 1 xfailed. New file 19/19 at np=1 and np=2.

Underworld development team with AI support from Claude Code

🤖 Generated with Claude Code

https://claude.ai/code/session_01MSGAFeA7qYXgkuw9ud8F2G

[Copilot]

🟡 Changes recommended

Unresolved moderate issues remain in swarm and mesh write handling, with an additional test-tier reporting nit.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Fixes silent symmetric-tensor .array write loss and deferred swarm updates.

Changes:

• Adds symmetric packing, mirroring, and conflict detection.
• Uses canonical swarm data during deferred writes.
• Adds parametrized regression tests.

File summaries

File	Summary and final findings
tests/test_0509_symmetric_tensor_array_writes.py	Adds regression coverage. Nit (1 vote): the tier_b marker excludes these tests from the reported tier_a run.
src/underworld3/swarm.py	Updates tensor packing and deferred writes. Moderate: setter validation is bypassed (3 votes); deferred reads use stale PETSc data (2 votes); global movement flags reject valid row-wise updates (1 vote).
src/underworld3/discretisation/discretisation_mesh_variables.py	Mirrors mesh symmetric-tensor writes. Moderate (1 vote): conflict detection must be applied per node rather than globally.

Review details

Suppressed comments (3)

src/underworld3/discretisation/discretisation_mesh_variables.py:2622

• The mesh implementation has the same whole-column boolean problem: a single assignment that updates opposite halves on different nodes is representable, but upper_moved and lower_moved become true globally and the helper raises based on unrelated rows. Apply the symmetric-pair decision per node, and raise only where both halves of the same node changed to different values.

                        upper_moved = not numpy.array_equal(after[:, i, j], before[:, i, j])
                        lower_moved = not numpy.array_equal(after[:, j, i], before[:, j, i])

src/underworld3/swarm.py:1043

• These array_equal calls reduce the moved state across all particles to one boolean. A valid single assignment can change [0, 1] for one particle and [1, 0] for another; both flags then become true and this raises even though each row has only one changed half and the symmetric storage can represent both values. Use row-wise moved/conflict masks, mirroring only rows where one half changed and rejecting only rows where both changed to different values.

                upper_moved = not np.array_equal(after[:, i, j], before[:, i, j])
                lower_moved = not np.array_equal(after[:, j, i], before[:, j, i])

tests/test_0509_symmetric_tensor_array_writes.py:26

• This new file is marked tier_b, so it is excluded from the reported level_1 and tier_a run. If these 19 regression tests are part of that validation, change the marker to tier_a; otherwise clarify in the test report that the new file was run separately.

pytestmark = [pytest.mark.level_1, pytest.mark.tier_b]

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.