Skip to content

Enable DUNE grid tests #903

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
SoilRos wants to merge 1 commit into
base: master
Choose a base branch
from
+22 −13
Draft

Enable DUNE grid tests #903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions opm/grid/cpgrid/CpGridData.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1577,8 +1577,8 @@ void CpGridData::distributeGlobalGrid(CpGrid& grid,

// Compute partition type for points
computePointPartitionType();
computeCommunicationInterfaces(noExistingPoints);

computeCommunicationInterfaces(noExistingPoints);
#else // #if HAVE_MPI
static_cast<void>(grid);
static_cast<void>(view_data);
Expand Down Expand Up @@ -2065,9 +2065,9 @@ int CpGridData::sharedFaceTag(const std::vector<std::array<int,3>>& startIJK_2Pa
assert(endIJK_2Patches.size() == 2);

int faceTag = -1; // 0 represents I_FACE, 1 J_FACE, and 2 K_FACE. Use -1 for no sharing face case.

if (patchesShareFace(startIJK_2Patches, endIJK_2Patches)) {

const auto& detectSharing = [](const std::vector<int>& faceIdxs, const std::vector<int>& otherFaceIdxs){
bool faceIsShared = false;
for (const auto& face : faceIdxs) {
Expand All @@ -2080,7 +2080,7 @@ int CpGridData::sharedFaceTag(const std::vector<std::array<int,3>>& startIJK_2Pa
}
return faceIsShared; // should be false here
};

const auto& [iFalse, iTrue, jFalse, jTrue, kFalse, kTrue] = this->getBoundaryPatchFaces(startIJK_2Patches[0], endIJK_2Patches[0]);
const auto& [iFalseOther, iTrueOther, jFalseOther, jTrueOther, kFalseOther, kTrueOther] =
this->getBoundaryPatchFaces(startIJK_2Patches[1], endIJK_2Patches[1]);
Expand Down Expand Up @@ -2668,11 +2668,11 @@ CpGridData::refinePatch(const std::array<int,3>& cells_per_dim, const std::array
bool CpGridData::mark(int refCount, const cpgrid::Entity<0>& element)
{
if (refCount == -1) {
OPM_THROW(std::logic_error, "Coarsening is not supported yet.");
return false; // Coarsening is not supported yet.
}
// Prevent refinement if the cell has a non-neighbor connection (NNC).
if (hasNNCs({element.index()}) && (refCount == 1)) {
OPM_THROW(std::logic_error, "Refinement of cells with face representing an NNC is not supported yet.");
return false; // Refinement of cells with face representing an NNC is not supported yet
}
assert((refCount == 0) || (refCount == 1)); // Do nothing (0), Refine (1), Coarsen (-1) not supported yet.
if (mark_.empty()) {
Expand All @@ -2699,7 +2699,7 @@ bool CpGridData::preAdapt()
if (local_empty)
mark_.resize(size(0));
}

// Detect the maximum mark across processes, and rewrite
// the local entry in mark_, i.e.,
// mark_[ element.index() ] = max{ local marks in processes where this element belongs to}.
Expand Down
6 changes: 6 additions & 0 deletions opm/grid/cpgrid/Entity.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,12 @@ unsigned int Entity<codim>::subEntities ( const unsigned int cc ) const
if ( cc == 3 ) { // Get number of corners of the element.
return 8;
}
if ( cc == 2 ) { // Get number of lines of the element.
return 12;
}
if ( cc == 1 ) { // Get number of faces of the element.
return 6;
}
Comment on lines +385 to +390

@blattms blattms Nov 6, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We clearly indicate in our capabilities that we do not support entities with these co-dimensions.
Shouldn't the check in DUNE honor that?

Even if we add this, then these numbers would not refelect any collapsed entities.

@SoilRos SoilRos Nov 6, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To run the test yes. If I remember correctly, the test checks that the number of intersections is higher than the number of subentites of codimension 1. As I wrote in the footnote of the PR description, this just matches what we already say with the geometry type.

From my understanding, this function is just the short-hand of calling referenceElement<double,dimension>(type()).size(codim). IMO, the current behavior, which is returning 0, is a bug. This is independent on whether the types for entities of lower codimension can be instantiated or not.

@SoilRos SoilRos Nov 13, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed privately, you are right in the sense that dune should not require this information if the grid does not have entities of that codimension enabled. I am thus proposing a check that confirms that entities and reference element counts coincide, and a fix that makes dune use the reference element instead of the entity information.

Naturally, this would be wrong one we try more degenerate code. But I think this is a good first step.

I added you as a reviewer in the dune side: See https://gitlab.dune-project.org/core/dune-grid/-/merge_requests/794.

}
return 0;
}
Expand Down
13 changes: 8 additions & 5 deletions tests/test_cpgrid.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ using Dune::referenceElement; //grid check assume usage of Dune::Geometry
#include <opm/input/eclipse/EclipseState/Grid/EclipseGrid.hpp>
#include <opm/input/eclipse/Parser/Parser.hpp>

#include <dune/common/version.hh>

#include <iostream>

template <class GridView>
Expand Down Expand Up @@ -91,16 +93,17 @@ template <class Grid>
void testGrid(Grid& grid, const std::string& name, const size_t nElem, const size_t nVertices)
{
typedef typename Grid::LeafGridView GridView;
/*

#if DUNE_VERSION_GTE(DUNE_GRID, 2, 10)
try {
gridcheck( grid );
}
catch ( const Dune::Exception& e)
{
std::cerr << "Warning: " << e.what() << std::endl;
}
*/
#endif

std::cout << name << std::endl;

testGridIteration( grid.leafGridView(), nElem );
Expand Down Expand Up @@ -166,7 +169,7 @@ int main(int argc, char** argv )

grid.processEclipseFormat(&ecl_grid, nullptr, false, false, false);
testGrid( grid, "CpGrid_ecl", 8, 27 );

const auto& grid_leafView = grid.leafGridView();
Dune::CartesianIndexMapper<Grid> grid_cartMapper = Dune::CartesianIndexMapper<Grid>(grid);
for (const auto& element: Dune::elements(grid_leafView)){
Expand All @@ -183,8 +186,8 @@ int main(int argc, char** argv )
}
std::cout << " " << '\n';
}


#endif

std::stringstream dgfFile;
Expand Down