CpGrid: build point communication interfaces from all points of a cell by hnil · Pull Request #1056 · OPM/opm-grid

hnil · 2026-07-31T08:42:41Z

On a faulted corner-point grid the hanging nodes along the fault are not canonical corners of the neighbouring cells, so point interfaces built from the 8 canonical corners never include them: they receive no message and vertex ownership is not a partition of unity across ranks (breaks vertex-based fields, e.g. mechanics).

First commit adds a test that fails on master (np 2/4); second builds the interfaces from all points of a cell, after which np 1/2/4 pass and distribution_test is unchanged.

One invariant worth stating: the per-cell point rows are traversal-order deduplicated, deliberately not sorted — the interface pairs sender/receiver rows positionally across ranks, and local point indices differ per rank, so sort+unique would silently reintroduce the bug.

blattms

I need to think about this a little. Did not look at the tests. So this is a first assessement. More to follow,

It is nice to have this fixed. We never needed communication between points before, hence this surely needed some love.

I think we should make sure that we do not add permanent addtional memory usage and maybe we make this more efficient, even.

blattms · 2026-08-10T07:45:19Z

+    const std::size_t nc = cell_to_point_.size();
+    std::vector<int> points;
+    for (std::size_t cell = 0; cell < nc; ++cell) {
+        points.clear();
+        const auto& faces = cell_to_face_[cpgrid::EntityRep<0>(cell, true)];
+        const int nf = faces.size();
+        for (int f = 0; f < nf; ++f) {
+            for (const auto& fv : face_to_point_[faces[f].index()]) {
+                if (std::find(points.begin(), points.end(), fv) == points.end()) {
+                    points.push_back(fv);
+                }
+            }
+        }
+        cell_to_allpoint_.appendRow(points.begin(), points.end());
+    }


I think the best place to do this is during (computeFace2Point)[https://github.com/hnil/opm-grid/blob/99aa1cc1f1761123ba5d75e3f94d4cde8d108ea9/opm/grid/cpgrid/CpGridData.cpp#L1241] namely during the last grid.scatterData which uses a FaceViaCellHandleWrapper<SparseTableDataHandle>

One possibility would be to optionally make SparseTableDataHandle populate your container with the values during its scatter method.

blattms · 2026-08-10T07:49:06Z

                mark_.resize(size(0));
        }
-
+       


We should not add trailing whitespace

blattms · 2026-08-10T08:00:12Z

+    /** @brief All points of each cell, i.e. every point of every face of the
+     *   cell, not only the eight canonical corners.  On corner-point grids
+     *   with hanging nodes the extra points are essential for building
+     *   complete codim-3 communication interfaces. */
+    Opm::SparseTable<int>             cell_to_allpoint_;


This is not really used after the set up and should be a local variable in some function

blattms · 2026-08-10T08:02:04Z

 {
    using DataType = typename Handle::DataType;
-    using C2PTable = std::vector< std::array<int,8> >;
+    using C2PTable = Opm::SparseTable<int>;


I don't think we even use the PointViaCellHandle. So maybe remove it?

Removed. Checked first that nothing uses it - not opm-grid, not opm-simulators, and not opm-flowgeomechanics - and PointViaCellWarner existed only as its base, so that and the translation unit holding the warning went too. 113 lines. All 84 opm-grid tests pass.

New 4-rank test (partition_of_unity_test): a 4x4x2 corner-point grid whose right half is shifted down by half a cell, partitioned so the fault plane coincides with a processor boundary. Fault-face processing produces hanging nodes -- face nodes that are not canonical corners of the neighbouring cell. The test communicates a codim-3 data handle over All_All_Interface and requires every non-interior vertex to receive at least one message. Before the all-points communication interface fix each rank reports two unreached hanging nodes ('Owner is not a partition of unity' in vertex-based mechanics assembly); with cell_to_allpoint_-based interfaces the test passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The codim-3 (point) communication interface and the point-via-cell data handle were built from cell_to_point_, i.e. only the eight canonical corners of each cell. On corner-point grids with hanging nodes a face can reference nodes that are not canonical corners of the neighbouring cell; such nodes never entered point_interfaces_, so they were invisible to codim-3 communication and their ownership failed to be a partition of unity across ranks (observed as 'Owner is not a partition of unity' in vertex-based mechanics assembly). Introduce cell_to_allpoint_ (SparseTable: every node of every face of each cell, sorted and deduplicated) and use it for the point interface and for PointViaCellHandleWrapper. AttributeDataHandle::fixedSize() now returns false since rows vary in length, and row iteration uses the SparseTable iterators. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…lone - cell_to_allpoint is now a local in the function that builds the point interfaces, so nothing is retained after set-up. This was the main objection: no permanent additional memory. - The duplicate check is a per-cell marker array instead of std::find, so building the rows is linear rather than quadratic in the points of a cell. Traversal order is untouched, which is the invariant the positional gather/scatter depends on. - PointViaCellHandleWrapper is left exactly as it was; its C2PTable typedef no longer needs changing. It has no users at all, but removing it is a separate cleanup. - Dropped the trailing whitespace this branch introduced. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

hnil · 2026-08-10T09:16:39Z

Thanks — three of four done, and the memory objection is fully addressed.

No persistent table. cell_to_allpoint is now a local in the function that builds the point interfaces; nothing is retained after set-up.
Cheaper, too. The duplicate check is a per-cell marker array instead of std::find, so the rows build in linear rather than quadratic time in a cell's points. Traversal order is unchanged — that is the invariant the positional gather/scatter depends on.
PointViaCellHandleWrapper left alone. You are right that nothing uses it — I checked, it has no callers anywhere in opm-grid. With the table local, my typedef change was unnecessary, so it is reverted and the struct is untouched. Removing it entirely seems like its own cleanup; happy to do that separately if you want it.
Trailing whitespace gone.

Not done: building this during computeFace2Point's last scatterData. That is the better home and I would rather you confirm the shape first — making SparseTableDataHandle optionally populate a cell-indexed container during scatter means threading that container through computeFace2Point's signature, which touches the distribution path rather than just this feature. Say the word and I will do it.

Rebased onto master (after #1051). np 1/2/4 pass, full opm-grid suite 84/84.

The file was added with a SINTEF header copied from a neighbour; the work is Equinor's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

hnil · 2026-08-11T12:02:03Z

What do I do with this @blattms

PointViaCellHandleWrapper has no users anywhere - not in opm-grid, not in opm-simulators, not in opm-flowgeomechanics - and PointViaCellWarner existed only to serve it, so both go, along with the translation unit that held the warning. Removing dead code rather than carrying it through the point-communication rework this PR performs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

hnil · 2026-08-21T08:02:13Z

jenkins build this please

hnil added the manual:bugfix This PR is a bug fix and should be noted in the manual label Jul 31, 2026

hnil requested review from akva2, atgeirr and blattms July 31, 2026 08:42

blattms requested changes Aug 10, 2026

View reviewed changes

hnil and others added 3 commits August 10, 2026 11:14

hnil force-pushed the pr/point-comm-partition-of-unity branch from 99aa1cc to a4c0e2e Compare August 10, 2026 09:16

tests: correct the copyright holder on the partition-of-unity test

28a19a2

The file was added with a SINTEF header copied from a neighbour; the work is Equinor's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

hnil requested a review from blattms August 20, 2026 12:07

Navigation Menu

Uh oh!

CpGrid: build point communication interfaces from all points of a cell - #1056

CpGrid: build point communication interfaces from all points of a cell#1056
hnil wants to merge 5 commits into
OPM:masterfrom
hnil:pr/point-comm-partition-of-unity

hnil commented Jul 31, 2026

Uh oh!

blattms left a comment

Uh oh!

blattms Aug 10, 2026

Uh oh!

blattms Aug 10, 2026

Uh oh!

blattms Aug 10, 2026 •

edited

Loading

Uh oh!

blattms Aug 10, 2026

Uh oh!

hnil Aug 20, 2026

Uh oh!

hnil commented Aug 10, 2026

Uh oh!

hnil commented Aug 11, 2026

Uh oh!

hnil commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

hnil commented Jul 31, 2026

Uh oh!

blattms left a comment

Choose a reason for hiding this comment

Uh oh!

blattms Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

blattms Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

blattms Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blattms Aug 10, 2026

Choose a reason for hiding this comment

Uh oh!

hnil Aug 20, 2026

Choose a reason for hiding this comment

Uh oh!

hnil commented Aug 10, 2026

Uh oh!

hnil commented Aug 11, 2026

Uh oh!

hnil commented Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

blattms Aug 10, 2026 •

edited

Loading