Skip to content
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.

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

Adds Verteq (Vertical Equilibrium) Grid #512

Open
wants to merge 27 commits into
base: master
Choose a base branch
from
Open

Conversation

rbe051
Copy link
Contributor

@rbe051 rbe051 commented Jan 27, 2021

The verteq (Vertical Equilibrium) is a revitalization of a former OPM project called verteq from some years back. It basically allows to colaps a column of grid cells from a corner point grid into a 2d cell while keeping a way of communicating between the two grids. This is used for longterm (>100 years) large scale (large reservoirs) CO2 simulations.

The above is copied from below to make it easier to find.

hafriis and others added 27 commits March 8, 2019 10:37
a proper DGF implementation for Polyhedron and Polygon.
@blattms
Copy link
Member

blattms commented Jan 27, 2021

Welcome to OPM. Thanks a lot for the contribution. I am sure this is very valuable but you could make our life a bit easier to assess and understand the value. Please think about doing this (would be very welcome):

  • Please add a more meaningful description. E.g. what is the verteq grid.
  • If possible split the PR. One for verteq, one for Polyhedral improvement/fixes etc.
  • Maybe you can even squash some of the commits (e.g. "Still not working" looks strange in the master branch)

@dr-robertk
Copy link
Member

dr-robertk commented Jan 27, 2021

The verteq (Vertical Equilibrium) is a revitalization of a former OPM project called verteq from some years back. It basically allows to colaps a column of grid cells from a corner point grid into a 2d cell while keeping a way of communicating between the two grids. This is used for longterm (>100 years) large scale (large reservoirs) CO2 simulations. The original code from Roland was based on UnstructuredGrid and thus with this PR we try to revitalize some of that code to be used in present day OPM. I think @totto82 can give you the full picture and @atgeirr can fill you in on the history.

@blattms blattms changed the title Verteq2 Adds Verteq Equilibrium Grid Jan 28, 2021
@blattms blattms changed the title Adds Verteq Equilibrium Grid Adds Verteq (Vertical Equilibrium) Grid Jan 28, 2021
Copy link
Member

@totto82 totto82 left a comment

Choose a reason for hiding this comment

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

Thanks for providing the PR. I know this is not purely your work, so thanks for the effort of trying to get it into the master code. Some cleanup and I think this can be merged. I agree with @blattms that a rebase -i to squash some of the commits would be helpful. Just give me a hint if you need guidance in what commits that needs be squashed together. Getting the VE code back in shape is a valuable contribution IMO.

@@ -0,0 +1,253 @@
#ifndef DUNE_POLYHEDRALGRID_MESH_HH
Copy link
Member

Choose a reason for hiding this comment

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

The polyhedralmesh.hh is according to @dr-robertk a WIP reimplementation of the unstructured grid and should be removed from this PR.

template <class Grid>
void testGrid(Grid& grid, const std::string& name, const size_t nElem, const size_t nVertices)
{
typedef typename Grid::LeafGridView GridView;
/*
//testVerteq( grid );

Copy link
Member

Choose a reason for hiding this comment

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

Please add back the gridcheck test for the standard grid. Also it would be useful to add the testVerteq test. But maybe not as part of testGrid but called independently.

@rbe051 rbe051 mentioned this pull request Feb 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants