Return geometry by value (alternative 4)

[SoilRos]

This is yet another version of how to return the geometry by value (see #885, #884, #891, and #334). Compared to the others, this should be the fastest one. The trick is made by not sharing the ownership of the corners, just a non-owning pointer. Additionally, since the geometryInFather needs to store its own data, I simply changed its type for Dune::AxisAlignedCubeGeometry which does the same job, but faster.

[SoilRos]

jenkins build this serial please

[SoilRos]

benchmark please

[ytelses]

Benchmark result overview:

Test	Configuration	Relative
opm-git	OPM Benchmark: drogon - Threads: 1	0.998
opm-git	OPM Benchmark: drogon - Threads: 8	1.015
opm-git	OPM Benchmark: punqs3 - Threads: 1	0.991
opm-git	OPM Benchmark: punqs3 - Threads: 8	0.998
opm-git	OPM Benchmark: smeaheia - Threads: 1	1.003
opm-git	OPM Benchmark: smeaheia - Threads: 8	0.987
opm-git	OPM Benchmark: spe10_model_1 - Threads: 1	1.008
opm-git	OPM Benchmark: spe10_model_1 - Threads: 8	0.994
opm-git	OPM Benchmark: flow_mpi_extra - Threads: 1	1.001
opm-git	OPM Benchmark: flow_mpi_extra - Threads: 8	0.992
opm-git	OPM Benchmark: flow_mpi_norne - Threads: 1	0.992
opm-git	OPM Benchmark: flow_mpi_norne - Threads: 8	1.003

• Speed-up = Total time master / Total time pull request. Above 1.0 is an improvement. *

View result details @ https://www.ytelses.com/opm/?page=result&id=3163

[SoilRos]

The table above seems wrong, the actual table reported in the website is:

Test	Configuration	Relative
opm-git	OPM Benchmark: drogon - Threads: 1	1.001
opm-git	OPM Benchmark: drogon - Threads: 8	1.212
opm-git	OPM Benchmark: punqs3 - Threads: 1	0.996
opm-git	OPM Benchmark: punqs3 - Threads: 8	1.012
opm-git	OPM Benchmark: smeaheia - Threads: 1	0.998
opm-git	OPM Benchmark: smeaheia - Threads: 8	1.075
opm-git	OPM Benchmark: spe10_model_1 - Threads: 1	1.009
opm-git	OPM Benchmark: spe10_model_1 - Threads: 8	0.995
opm-git	OPM Benchmark: flow_mpi_extra - Threads: 1	1.002
opm-git	OPM Benchmark: flow_mpi_extra - Threads: 8	1.13
opm-git	OPM Benchmark: flow_mpi_norne - Threads: 1	1.002
opm-git	OPM Benchmark: flow_mpi_norne - Threads: 8	1.078

• Speed-up = Total time master / Total time pull request. Above 1.0 is an improvement. *

[SoilRos]

Locally (Apple M3 Max), I get simular results:

To me, this suggests that the assembly is already copying the geometry somewhere and we have been paying the price of the shared pointer reference counting.

[atgeirr]

Just a question: how can Dune::AxisAlignedCubeGeometry be used, our grid cells are not axis aligned (except for artificial test cases)?

[SoilRos]

@atgeirr geometryInFather() transforms the reference element of the child element onto the reference element of the father element. Both are axial aligned cube geometries even if the original element of the father/child element is a strange geometry, even if several corners are collapsed into one.

[aritorto]

@atgeirr @SoilRos I just added a small test to illustrate what might be obvious: a deformed cell (i.e. a cell with collapsed corners and therefore fewer than 8 corners) does not map to the entire unit cube (#1065).

With the current code in master, refining a Geometry<3,3> assumes that the parent cell geometry is a hexahedron, so its 8 corners map perfectly onto the unit cube. This means that, at the moment, we don't support refinement of deformed cells (cells with fewer than 8 corners).

If the pillars are skewed but the cell is still a hexahedron, however, it can be refined — see [this test](https://github.com/OPM/opm-grid/blob/master/tests/cpgrid/lgr/refine_hexahedron_with_non_rectangular_faces_test.cpp).

Also, geometryInFather() (both here and in master) will throw if called for a deformed cell, since such a cell cannot have a father and therefore cannot have a geometryInFather representation.

[atgeirr]

I have looked at this again and think it looks good and should probably be merged. I do have a few questions first:

• The Entity<...>geometry() method now returns by value as the current Dune approach expects. But is it also necessary or expected to return for example center() by value? That seems to me it would require unnecessary duplicate copies.
• The performance results posted are interesting, and surprising! Why is there an improvement for higher process counts and not for lower? Have you done any profiling to confirm this is related to the change in this PR? Did you perform multiple experiments to rule out differences due to different and random background task loads etc.? Also, it says default, I assume that means 2 threads per process, which may in part explain why shared_ptr manipulation could have a higher cost, but it also seems a bit high on a M3 max with 10 or 12 performance cores (not sure which version you have).

Currently unmergeable due to conflict, please resolve.

[SoilRos]

The Entity<...>geometry() method now returns by value as the current Dune approach expects. But is it also necessary or expected to return for example center() by value? That seems to me it would require unnecessary duplicate copies.

Good question! Initially, I implemented this to ensure that I has not leaving dangling references behind during testing. For compiled objects, definitely yes, this is more expensive as the ABI of the function needs to write the signature explicitly. For templated code where the function may be inserted in-place, I do not know what are optimizations can the compiler do with either version. From what I can gather, these functions are compiled into binaries for the iteration of center of elements (e.g. via faceCentroid or vertexPosition) while other usages of geometry get the actual function. I do not know which version ends up in the assembly. Since this is not a requirement for the Dune interface, I extracted that into another commit (SoilRos@c3864d6) and left it out of this PR. In any case, in the tests below you can see that it makes a minimal impact on norne.

The performance results posted are interesting, and surprising! Why is there an improvement for higher process counts and not for lower? Have you done any profiling to confirm this is related to the change in this PR? Did you perform multiple experiments to rule out differences due to different and random background task loads etc.? Also, it says default, I assume that means 2 threads per process, which may in part explain why shared_ptr manipulation could have a higher cost, but it also seems a bit high on a M3 max with 10 or 12 performance cores (not sure which version you have).

Thanks for asking, I usually check that results are statistically significant. I am confident that the sequential cases are statistically significant but I probably have done a mistake here because I cannot directly reproduce the ones with higher process count. I have 10 performance cores and these results are not explained by the multithreading because I always run benchmarks with 1 thread per process (this is the default in my machine, I do not know why but I prefer this for measurements anyways). So I just re-ran the experiments (several times) for 3 cases:

• master: OPM/opm-common@a3bdcb4, 0f95d63, OPM/opm-simulators@f2a6ad1.
• geo: master+changes that return Entity::geometry() by value.
• coords: master+changes that return Entity::geometry() and Geometry::center() by value.

Case	MPI	Threads	Runs	Mean(s)	Std(s)	Min(s)	Max(s)
master	10	1	5	46.88	0.33	46.65	47.46
geo	10	1	6	46.86	0.17	46.57	47.02
coords	10	1	6	47.04	0.40	46.55	47.63

Currently unmergeable due to conflict, please resolve.

Done 👍

[SoilRos]

jenkins build this serial please

[SoilRos]

benchmark please

[ytelses]

Benchmark result overview:

Test	Configuration	Relative

• Speed-up = Total time master / Total time pull request. Above 1.0 is an improvement. *

FAILED RUNS:
OPM Benchmark: drogon - Threads: 1 - 2026-09-02 Master Reference: The test run did not produce a result. E: mpirun was unable to launch the specified application as it could not access
OPM Benchmark: drogon - Threads: 1 - opm-grid 1055: Return geometry by value alternative 4: The test run did not produce a result. The test run did not produce a result.
OPM Benchmark: drogon - Threads: 8 - 2026-09-02 Master Reference: The test run did not produce a result. E: mpirun was unable to launch the specified application as it could not access
OPM Benchmark: drogon - Threads: 8 - opm-grid 1055: Return geometry by value alternative 4: The test run did not produce a result. The test run did not produce a result.
OPM Benchmark: punqs3 - Threads: 1 - 2026-09-02 Master Reference: The test run did not produce a result. E: mpirun was unable to launch the specified application as it could not access
OPM Benchmark: punqs3 - Threads: 1 - opm-grid 1055: Return geometry by value alternative 4: The test run did not produce a result. The test run did not produce a result.
OPM Benchmark: punqs3 - Threads: 8 - 2026-09-02 Master Reference: The test run did not produce a result. E: mpirun was unable to launch the specified application as it could not access
OPM Benchmark: punqs3 - Threads: 8 - opm-grid 1055: Return geometry by value alternative 4: The test run did not produce a result. The test run did not produce a result.
OPM Benchmark: smeaheia - Threads: 1 - 2026-09-02 Master Reference: The test run did not produce a result. E: mpirun was unable to launch the specified application as it could not access
OPM Benchmark: smeaheia - Threads: 1 - opm-grid 1055: Return geometry by value alternative 4: The test run did not produce a result. The test run did not produce a result.
OPM Benchmark: smeaheia - Threads: 8 - 2026-09-02 Master Reference: The test run did not produce a result. E: mpirun was unable to launch the specified application as it could not access
OPM Benchmark: smeaheia - Threads: 8 - opm-grid 1055: Return geometry by value alternative 4: The test run did not produce a result. The test run did not produce a result.
OPM Benchmark: spe10_model_1 - Threads: 1 - 2026-09-02 Master Reference: The test run did not produce a result. E: mpirun was unable to launch the specified application as it could not access
OPM Benchmark: spe10_model_1 - Threads: 1 - opm-grid 1055: Return geometry by value alternative 4: The test run did not produce a result. The test run did not produce a result.
OPM Benchmark: spe10_model_1 - Threads: 8 - 2026-09-02 Master Reference: The test run did not produce a result. E: mpirun was unable to launch the specified application as it could not access
OPM Benchmark: spe10_model_1 - Threads: 8 - opm-grid 1055: Return geometry by value alternative 4: The test run did not produce a result. The test run did not produce a result.
OPM Benchmark: flow_mpi_extra - Threads: 1 - 2026-09-02 Master Reference: The test run did not produce a result. E: mpirun was unable to launch the specified application as it could not access
OPM Benchmark: flow_mpi_extra - Threads: 1 - opm-grid 1055: Return geometry by value alternative 4: The test run did not produce a result. The test run did not produce a result.
OPM Benchmark: flow_mpi_extra - Threads: 8 - 2026-09-02 Master Reference: The test run did not produce a result. E: mpirun was unable to launch the specified application as it could not access
OPM Benchmark: flow_mpi_extra - Threads: 8 - opm-grid 1055: Return geometry by value alternative 4: The test run did not produce a result. The test run did not produce a result.
OPM Benchmark: flow_mpi_norne - Threads: 1 - 2026-09-02 Master Reference: The test run did not produce a result. E: mpirun was unable to launch the specified application as it could not access
OPM Benchmark: flow_mpi_norne - Threads: 1 - opm-grid 1055: Return geometry by value alternative 4: The test run did not produce a result. The test run did not produce a result.
OPM Benchmark: flow_mpi_norne - Threads: 8 - 2026-09-02 Master Reference: The test run did not produce a result. E: mpirun was unable to launch the specified application as it could not access
OPM Benchmark: flow_mpi_norne - Threads: 8 - opm-grid 1055: Return geometry by value alternative 4: The test run did not produce a result. The test run did not produce a result.

View result details @ https://www.ytelses.com/opm/?page=result&id=3198

[SoilRos]

jenkins build this serial please

[SoilRos]

The tests seem to be broken since August 7th, so it's not related to this PR.

[SoilRos]

jenkins build this serial please