From 87c812baaa73b3b211c9948e395ff4382730f187 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Santiago=20Ospina=20De=20Los=20R=C3=ADos?= Date: Thu, 13 Nov 2025 17:48:59 +0100 Subject: [PATCH 1/2] Split the id and idLevelZero interface The id method is being overloaded for two cases, one for the Entity and another one for EntityRep. The problem is that the two overloads do vastly different things and since the Entity inherits from EntityRep, it is likely that a cast to the base case changes the behavior without anyone noticing. This, this patch splits the id function into id and idLevelZero to clarify that the id function is only valid for full entities of type Entity and not its base class EntityRep. --- opm/grid/cpgrid/CpGridData.cpp | 15 ++++++++------- opm/grid/cpgrid/Entity.hpp | 2 +- opm/grid/cpgrid/Indexsets.cpp | 6 ------ opm/grid/cpgrid/Indexsets.hpp | 16 ++++++++++------ tests/cpgrid/distribution_test.cpp | 14 ++++++-------- tests/cpgrid/lgr/id_entity_entityrep_test.cpp | 2 +- 6 files changed, 26 insertions(+), 29 deletions(-) diff --git a/opm/grid/cpgrid/CpGridData.cpp b/opm/grid/cpgrid/CpGridData.cpp index 92a2b24c0d..da9939b93d 100644 --- a/opm/grid/cpgrid/CpGridData.cpp +++ b/opm/grid/cpgrid/CpGridData.cpp @@ -587,7 +587,7 @@ struct Cell2PointsDataHandle const auto& points = globalCell2Points_[i]; std::ranges::for_each(points, [&buffer, this](const int& point) - { buffer.write(globalIds_.id(EntityRep<3>(point, true))); }); + { buffer.write(globalIds_.idLevelZero(EntityRep<3>(point, true))); }); for (const auto& point: globalAdditionalPointIds_[i]) { buffer.write(point); @@ -707,7 +707,7 @@ struct SparseTableDataHandle const auto& entries = global_[t.index()]; std::ranges::for_each(entries, [&buffer, this](const DataType& i) - { buffer.write(globalIds_.id(EntityRep<3>(i, true))); }); + { buffer.write(globalIds_.idLevelZero(EntityRep<3>(i, true))); }); } template void scatter(B& buffer, const T& t, std::size_t ) @@ -779,7 +779,7 @@ struct OrientedEntityTableDataHandle std::ranges::for_each(entries, [&buffer, this](const ToEntity& i) { - int id = globalIds_->id(i); + int id = globalIds_->idLevelZero(i); if (!i.orientation()) { id = ~id; } @@ -853,8 +853,9 @@ struct IndexSet2IdSet for (const auto& entry: indexSet) map_[entry.local()] = entry.global(); } - template - int id(const T& t) const + + template + int idLevelZero(const EntityRep& t) const { return map_[t.index()]; } @@ -1371,7 +1372,7 @@ std::vector > computeAdditionalFacePoints(const std::vector(point,true))); + additionalFacePoints[c].insert(globalIds.idLevelZero(EntityRep<3>(point,true))); } } } @@ -1475,7 +1476,7 @@ std::map computeCell2Point(const CpGrid& grid, createInterfaceList(procCellLists, globalCell2Points, globalAdditionalPoints, [&globalIds](int i){ - return globalIds.id(EntityRep<3>(i, true)); + return globalIds.idLevelZero(EntityRep<3>(i, true)); }, globalMap2Local, pointInterfaces[procCellLists.first]); diff --git a/opm/grid/cpgrid/Entity.hpp b/opm/grid/cpgrid/Entity.hpp index ce716e6913..59590359e7 100644 --- a/opm/grid/cpgrid/Entity.hpp +++ b/opm/grid/cpgrid/Entity.hpp @@ -387,7 +387,7 @@ unsigned int Entity::subEntities ( const unsigned int cc ) const return 8; } } - return 0; + DUNE_THROW(NotImplemented, "subEntities not implemented for this codimension"); } template diff --git a/opm/grid/cpgrid/Indexsets.cpp b/opm/grid/cpgrid/Indexsets.cpp index 1ea1593126..efbeec40fe 100644 --- a/opm/grid/cpgrid/Indexsets.cpp +++ b/opm/grid/cpgrid/Indexsets.cpp @@ -48,8 +48,6 @@ IndexSet::IndexType IndexSet::subIndex(const cpgrid::Entity<0>& e, int i, unsign { switch(cc) { case 0: return index(e.subEntity<0>(i)); - case 1: return index(e.subEntity<1>(i)); - // case 2: return index(e.subEntity<2>(i)); // Not supported in this grid case 3: return index(e.subEntity<3>(i)); default: OPM_THROW(std::runtime_error, "Codimension " + std::to_string(cc) + " not supported."); @@ -60,8 +58,6 @@ IdSet::IdType IdSet::subId(const cpgrid::Entity<0>& e, int i, int cc) const { switch (cc) { case 0: return id(e.subEntity<0>(i)); - case 1: return id(e.subEntity<1>(i)); - // case 2: return id(e.subEntity<2>(i)); // Not supported in this grid case 3: return id(e.subEntity<3>(i)); default: OPM_THROW(std::runtime_error, "Cannot get subId of codimension " + std::to_string(cc)); @@ -75,8 +71,6 @@ LevelGlobalIdSet::IdType LevelGlobalIdSet::subId(const cpgrid::Entity<0>& e, int switch (cc) { case 0: return id(e.subEntity<0>(i)); - //case 1: return id(*e.subEntity<1>(i)); - //case 2: return id(*e.subEntity<2>(i)); case 3: return id(e.subEntity<3>(i)); default: OPM_THROW(std::runtime_error, "Cannot get subId of codimension " + std::to_string(cc)); diff --git a/opm/grid/cpgrid/Indexsets.hpp b/opm/grid/cpgrid/Indexsets.hpp index c80211c40d..06624b4108 100644 --- a/opm/grid/cpgrid/Indexsets.hpp +++ b/opm/grid/cpgrid/Indexsets.hpp @@ -224,13 +224,11 @@ namespace Dune { if constexpr (cd == 0) return computeId_cell(e); - else if constexpr (cd == 1) - return computeId(e); else if constexpr (cd == 3) return computeId_point(e); else static_assert(AlwaysFalse>::value, - "IdSet::id not implemented for codims other thatn 0, 1, and 3."); + "IdSet::id not implemented for codims other than 0, and 3."); } template @@ -240,11 +238,14 @@ namespace Dune } template - IdType id(const cpgrid::EntityRep& e) const + IdType idLevelZero(const cpgrid::EntityRep& e) const { return computeId(e); } + template + IdType idLevelZero(const Entity& e) const = delete; + /// return id of intersection (here face number) IdType id( const cpgrid::Intersection& intersection ) const { @@ -414,14 +415,17 @@ namespace Dune } template - IdType id(const EntityRep& e) const + IdType idLevelZero(const EntityRep& e) const { if(idSet_) - return idSet_->id(e); + return idSet_->idLevelZero(e); else return this->template getMapping()[e.index()]; } + template + IdType idLevelZero(const Entity& e) const = delete; + template IdType id(const EntityType& e) const { diff --git a/tests/cpgrid/distribution_test.cpp b/tests/cpgrid/distribution_test.cpp index c00d26075b..f06bb703d6 100644 --- a/tests/cpgrid/distribution_test.cpp +++ b/tests/cpgrid/distribution_test.cpp @@ -500,8 +500,6 @@ BOOST_AUTO_TEST_CASE(compareWithSequential) using namespace Dune::cpgrid; auto face = grid.cellFace(eIt->index(), f); auto seqFace = seqGrid.cellFace(seqEIt->index(), f); - BOOST_REQUIRE(idSet.id(Dune::createEntity<1>(grid, face, true)) == - seqIdSet.id(Dune::createEntity<1>(seqGrid, seqFace, true))); int vertices = grid.numFaceVertices(face); BOOST_REQUIRE(vertices == seqGrid.numFaceVertices(seqFace)); for (int v = 0; v < vertices; ++v) @@ -579,11 +577,11 @@ BOOST_AUTO_TEST_CASE(PartitionTest) } } MPI_Barrier(MPI_COMM_WORLD); - + if (size > 2) { MPI_Comm_free(&twocom); - } + } #endif } @@ -638,7 +636,7 @@ BOOST_AUTO_TEST_CASE(PartitionTestWithCorners) } std::cout<<"done interior "<<(int)Dune::InteriorEntity<<" overlap "<<(int)Dune::OverlapEntity <<" border "<<(int)Dune::BorderEntity<<" front "<<(int)Dune::FrontEntity< 2) { MPI_Comm_free(&twocom); - } + } #endif } diff --git a/tests/cpgrid/lgr/id_entity_entityrep_test.cpp b/tests/cpgrid/lgr/id_entity_entityrep_test.cpp index c10933238c..c6da59d830 100644 --- a/tests/cpgrid/lgr/id_entity_entityrep_test.cpp +++ b/tests/cpgrid/lgr/id_entity_entityrep_test.cpp @@ -86,7 +86,7 @@ std::string coincideString(bool coincide) template auto idRep(const IdSet& idSet, const EntityRep& entityRep) { - return idSet.id(entityRep); + return idSet.idLevelZero(entityRep); } void entityRepIndexAndEntityIndexCoincide(const Dune::CpGrid& grid) From b9516f9b07d311c2fba0733d2818685ec6468127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Santiago=20Ospina=20De=20Los=20R=C3=ADos?= Date: Thu, 19 Feb 2026 14:12:33 +0100 Subject: [PATCH 2/2] Export supported entity types via a codim traits This addresses two issues: * The compiler warns that the name Entity is being overloaded by the traits of the Entity class. * The dune-grid interface in version 2.10 requires the Codim traits to be SFINAE friendly in the sense that missing codimension need to be a Substitution Failure (the SF part of SFINAE). --- opm/grid/cpgrid/Entity.hpp | 41 ++++++++++++++++++++++++++++++----- opm/grid/cpgrid/Indexsets.hpp | 27 +++++++---------------- 2 files changed, 44 insertions(+), 24 deletions(-) diff --git a/opm/grid/cpgrid/Entity.hpp b/opm/grid/cpgrid/Entity.hpp index 59590359e7..f60e23bc0f 100644 --- a/opm/grid/cpgrid/Entity.hpp +++ b/opm/grid/cpgrid/Entity.hpp @@ -57,6 +57,41 @@ namespace Dune namespace cpgrid { +template +class Entity; + +namespace Impl { +// Forward declaration of traits class for codimensions. +template +struct CodimTraits; + +// Specialization of traits class for codimension 0 (cells). +template<> +struct CodimTraits<0> +{ + using Entity = ::Dune::cpgrid::Entity<0>; +}; + +#if DUNE_VERSION_LT(DUNE_GRID, 2, 11) +// Specialization of traits class for codimension 1 (faces). +// This is only needed for DUNE < 2.11, as the face entity type was needed for the VTK writer, which was fixed in DUNE 2.11. +// See: https://gitlab.dune-project.org/core/dune-grid/-/merge_requests/793 +template<> +struct CodimTraits<1> +{ + using Entity = ::Dune::cpgrid::Entity<1>; +}; +#endif + +// Specialization of traits class for codimension 3 (vertices). +template<> +struct CodimTraits<3> +{ + using Entity = ::Dune::cpgrid::Entity<3>; +}; + +} + template class Geometry; template class Iterator; class IntersectionIterator; @@ -93,11 +128,7 @@ class Entity : public EntityRep /** \brief Export supported entity types */ template - struct Codim - { - using Entity = ::Dune::cpgrid::Entity; - }; - + using Codim = typename Impl::CodimTraits; typedef cpgrid::Geometry<3-codim,3> Geometry; typedef Geometry LocalGeometry; diff --git a/opm/grid/cpgrid/Indexsets.hpp b/opm/grid/cpgrid/Indexsets.hpp index 06624b4108..8e1b4910f0 100644 --- a/opm/grid/cpgrid/Indexsets.hpp +++ b/opm/grid/cpgrid/Indexsets.hpp @@ -38,6 +38,7 @@ along with OPM. If not, see . #include #include +#include "Entity.hpp" #include "GlobalIdMapping.hpp" #include "Intersection.hpp" @@ -62,11 +63,8 @@ namespace Dune static constexpr int dimension = 3; /** \brief Export supported entity types */ - template - struct Codim - { - typedef cpgrid::Entity< cc > Entity; - }; + template + using Codim = typename Impl::CodimTraits; /// @brief /// @todo Doc me! @@ -208,11 +206,8 @@ namespace Dune static constexpr int dimension = 3; /** \brief Export supported entity types */ - template - struct Codim - { - using Entity = ::Dune::cpgrid::Entity; - }; + template + using Codim = typename Impl::CodimTraits; explicit IdSet(const CpGridData& grid) : grid_(grid) @@ -378,11 +373,8 @@ namespace Dune static constexpr int dimension = 3; /** \brief Export supported entity types */ - template - struct Codim - { - using Entity = ::Dune::cpgrid::Entity; - }; + template + using Codim = typename Impl::CodimTraits; void swap(std::vector& cellMapping, std::vector& faceMapping, @@ -497,10 +489,7 @@ namespace Dune /** \brief Export supported entity types */ template - struct Codim - { - using Entity = ::Dune::cpgrid::Entity; - }; + using Codim = typename Impl::CodimTraits; explicit GlobalIdSet(const CpGridData& view);