Revert PR #172 (integrals regression) + xfail CellWiseIntegral parity tests

[lmoresi]

Summary

PR #172 ("Perf: clone DM in Integral.evaluate() (3× speedup; closes #171 narrative)") introduced a regression that broke 4 tests in tests/test_0501_integrals.py. CI on the merged commit (fe05c63) and on development's HEAD shows these failures. This PR reverts #172 to restore correctness, and adds parity test coverage for the sister CellWiseIntegral class — which the original PR claimed worked but never actually has.

What broke

The change in #172 was:

dmc = self.mesh.dm.clone()                # PETSc DMClone does not copy fields
fec = FE.createDefault(dim, 1, False, -1)
dmc.setField(0, fec)                      # clone now has just one P1 field
dmc.createDS()
PetscDSSetObjective(dmc.getDS(), 0, fn)
DMPlexComputeIntegralFEM(dmc, cgvec, ...) # cgvec packed for mesh.dm's multi-field layout

The integrand reads through dmc's single-field DS, but the vector being integrated (cgvec) was packed for mesh.dm's full multi-field layout. The DOFs read are wrong, producing ~2× over-counts on simple cases and NaN/0 on derivative integrands. All four failing tests are explained by this mismatch.

Performance ground truth (#171)

The PR's stated motivation was to fix linear-time growth in t_vol over long convection runs (5 s → 833 s over ~3000 calls). The PR itself notes it could not reproduce the slowdown locally (200 calls × 100-cell mesh stayed flat). The fix was speculative; correctness on the failing tests was lost in exchange for an unverified perf benefit. Issue #171 should be reopened with a real reproducer before any further attempt.

What's in this PR

1. Revert fe05c63 — restores the pre-Perf: clone DM in Integral.evaluate() (3× speedup; closes #171 narrative) #172 Integral.evaluate() (uses mesh.dm + mesh.dm.getDS() directly).

2. CellWiseIntegral parity tests (xfail) — the sister class had zero test coverage. While auditing, found two latent bugs:

   • self.dim typo (it should be self.mesh.dim) — fixed; the class would crash with AttributeError before now.
   • With the typo fixed, CellWiseIntegral exhibits the same field-cloning bug as Perf: clone DM in Integral.evaluate() (3× speedup; closes #171 narrative) #172 — sum of per-cell volumes returns 2.0 instead of 1.0 on the unit square.

   The two new tests (test_cellwise_integrate_constants, test_cellwise_integrate_meshvar) are marked xfail with a clear reason. They will start passing when CellWiseIntegral.evaluate() is rewritten to use the same mesh.dm + getDS() pattern that the reverted Integral already uses.

3. Inline TODO(BUG) in CellWiseIntegral.evaluate() pointing at the same write-up.

Test plan

• pytest tests/test_0501_integrals.py -v — 9 passed, 3 xfailed (1 pre-existing + 2 new with documented reasons)
• CI shows the previously-failing 4 integrals tests now pass
• Reopen issue Volume integral evaluation time grows linearly across long runs (~80x in 3000 calls) #171 with a note that the linear-growth issue still needs a real reproducer; until then, correctness wins

Underworld development team with AI support from Claude Code

[Copilot]

Pull request overview

Reverts PR #172’s DM-cloning approach in Integral.evaluate() to restore correctness (avoiding DOF layout mismatches), and adds xfail parity tests documenting that CellWiseIntegral.evaluate() currently produces incorrect results due to the same DM/Vec layout mismatch.

Changes:

• Reverted Integral.evaluate() to compute integrals directly on mesh.dm/mesh.dm.getDS() (instead of cloning and creating a single-field DS per call).
• Added CellWiseIntegral parity tests (xfail) for constants and a mesh variable integrand.
• Fixed a CellWiseIntegral dimension typo (self.dim → self.mesh.dim) and added an inline TODO describing the known bug.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
src/underworld3/cython/petsc_maths.pyx	Reverts Integral.evaluate() to shared DM/DS path; updates CellWiseIntegral.evaluate() to use self.mesh.dim and adds TODO about known DM/Vec mismatch.
tests/test_0501_integrals.py	Adds xfail parity tests for CellWiseIntegral to document current incorrect behavior.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.