Level global cell for CpGrid with LGRs #762

aritorto · 2024-09-25T10:15:55Z

This PR is relevant for including wells on LGRs (CpGrid). Potentially relevant for output files too.

For CpGrid with LGRs, we determine the global_cell_ values for refined level grids.
If an LGR has NX * NY * NZ dimension (amount of cells in each direction x,y,z respectively), then each refined cell can be associated with a (local) IJK and its (local) Cartesian index.
For a leaf grid view, we assign the global_cell_ values of either the parent cell or the equivalent cell from level zero.

Not relevant for the Reference Manual.

aritorto · 2024-09-25T10:16:47Z

jenkins build this please

aritorto · 2024-09-25T13:23:25Z

jenkins build this please

aritorto · 2024-09-26T12:49:01Z

jenkins build this please

aritorto · 2024-09-26T13:21:41Z

@blattms maps for global cell from level grids and the leaf index set have been added. Could you take a look?

@bska do you think these changes - apart from potential use for wells location on LGRs - are also relevant for including LGRs in output files?

blattms · 2024-09-27T11:32:27Z

If we refine without LGRs, what size and values does globalCell have for the levels?

blattms

Looks good.
Just a few suggestions for better code reuse.

There are also comments on checks that won't hold if there are inactive cells. Maybe that is tested elsewhere and irrelevant?

opm/grid/cpgrid/CartesianIndexMapper.hpp

opm/grid/cpgrid/CpGrid.cpp

blattms · 2024-09-27T11:48:37Z

opm/grid/cpgrid/CpGrid.cpp

+        std::array<int,3> childIJK = {0,0,0};
+
+        childIJK[0] = idx_in_parent_cell % cells_per_dim[0];
+        idx_in_parent_cell -= childIJK[0]; // k*cells_per_dim[0]*cells_per_dim[1] + j*cells_per_dim[0]
+        idx_in_parent_cell /= cells_per_dim[0]; // k*cells_per_dim[1] + j
+        childIJK[1] = idx_in_parent_cell % cells_per_dim[1];
+        idx_in_parent_cell -= childIJK[1]; // k*cells_per_dim[1]
+        childIJK[2] = idx_in_parent_cell /cells_per_dim[1];


Such computations are done a lot. It might be good to refactor them out to a separate function and reuse it elsewhere (e.g. CpGridData::getIJK)

This is now in CpGridData::getInParentCellIJK()

opm/grid/cpgrid/CpGrid.cpp

opm/grid/cpgrid/CpGridData.cpp

blattms · 2024-09-27T12:01:43Z

tests/cpgrid/adapt_cpgrid_test.cpp

+        auto itMin = std::min_element((data.back() -> global_cell_).begin(),  (data.back()-> global_cell_).end());
+        auto itMax = std::max_element((data.back() -> global_cell_).begin(),  (data.back() -> global_cell_).end());
+        BOOST_CHECK_EQUAL( *itMin, 0);
+        BOOST_CHECK_EQUAL( *itMax, data.front()-> size(0) -1);


Maybe change this test to work also for grid with inactive cells?
You could check that each value is between zero and the maximum cartesian index possible.

There are checks in opm-grid/tests/cpgrid/inactiveCells_lgr_test.cpp, e.g. here. However, #761 intends to remove all the implementation details, in particular the previous check.

In this case, we can replace data.front()-> size(0)-1 by grid.logicalCartesianSize()[0]...[1]....*[2]

blattms · 2024-09-27T12:03:58Z

tests/cpgrid/adapt_cpgrid_test.cpp

+            auto itMinLevel = std::min_element((data[level] -> global_cell_).begin(),  (data[level] -> global_cell_).end());
+            auto itMaxLevel = std::max_element((data[level] -> global_cell_).begin(),  (data[level] -> global_cell_).end());
+            BOOST_CHECK_EQUAL( *itMinLevel, 0);
+            BOOST_CHECK_EQUAL( *itMaxLevel, data[level]-> size(0) -1);


Same here and below. There could be inactive parent cells in the region to refine and hence missing kids (in the future?).

Now the check is done with inequalities, meaning that the maximum value of global cell should be between 0 and the maximum Cartesian Index (the later might vary for each LGR, and it's equal to the maximum Cartesian Index from level zero for the leaf grid view)

aritorto · 2024-09-30T06:00:29Z

If we refine without LGRs, what size and values does globalCell have for the levels?

Good point. I didn't comment on that in the PR description.

When we refine without LGRs, the size of global_cell_ coincides with the total amount of cells on the level grid/leaf grid view. For refined level grids, the values are 0,1,2,..., total amount of cells in that refined level grid.

For the leaf grid view, both refine-methods (with or without LGRs) do exactly the same: <leaf_grid_view>.global_cell_[ leaf cell index ] = global_cell_ value of the oldest ancestor from level zero.

One idea to make these two refine-approaches compatible is to detect minimum and maximums ijk values of marked cells, and mimic what's done for LGRs in the without-LGRs refinement, with these "fake startIJK and endIJK"

blattms · 2024-09-30T08:23:03Z

One idea to make these two refine-approaches compatible is to detect minimum and maximums ijk values of marked cells, and mimic what's done for LGRs in the without-LGRs refinement, with these "fake startIJK and endIJK"

I don't think we should make them compatible as they are not. cartesian indices on the level only makes sense for LGRs. In the long term we should probably throw if useers use them in adaptively refined grids. Let's postpone this, though,

aritorto · 2024-09-30T11:05:25Z

There are also comments on checks that won't hold if there are inactive cells. Maybe that is tested elsewhere and irrelevant?

Now there are a few lines in inactiveCell_test.cpp checking that the max value of global cell is below the max Cartesian Index for level grids, and for the leaf (which corresponds to the max Cartesian Index value of level zero grid)

aritorto · 2024-09-30T11:09:02Z

jenkins build this please

aritorto · 2024-09-30T11:09:42Z

@blattms thanks for your feedback! (which is now incorporated)

blattms · 2024-09-30T13:59:07Z

opm/grid/cpgrid/CpGridData.hpp

+    ///                                     (total amount of children of a parent cell minus one since starting index is zero).
+    /// @param [in] cells_per_dim
+    /// @param [out] ijk                    In-parent-cell Cartesian index triplet
+    void getInParentCellIJK(int idx_in_parent_cell, const std::array<int,3>& cells_per_dim,  std::array<int,3>& ijk) const


Isn't this much more general than the name and the documentation suggests? It seems to convert a cartesian index to ijk and has no relation to refinement. You just happen to use it during refinement, currently. Why not simply name it getIJK?

In addition we could just return the ijk directly and only have two parameters.
Then you can do this in line 306: ijk = getIK(c, logical_cartesian_size_); and that method is done.

It sounds doable. I kept both methods since CpGrid::getIJK uses current_view_data_ ->getIJK (which is CpGridData::getIJK), and the integer argument "c" then it's used to get global_cell_[c] and then with this new integer, the "usual repeated deduction of ijk is done"

It's probably your comment combined with ijk = getIJK(global_cell_[c], logical_cartesian_size_)

aritorto · 2024-10-01T05:56:13Z

jenkins build this please

aritorto force-pushed the globalCellLgr branch from e9e6a57 to 694547c Compare September 25, 2024 10:16

aritorto force-pushed the globalCellLgr branch from 694547c to c70338f Compare September 25, 2024 13:23

aritorto force-pushed the globalCellLgr branch 2 times, most recently from 9ba2308 to f1f25d0 Compare September 26, 2024 12:46

aritorto changed the title ~~Modify global_cell_ for lgrs~~ Local Cartesian Indices for LGRs Sep 26, 2024

aritorto mentioned this pull request Sep 26, 2024

Extend AluCartesianIndexMapper due to speciatilization of CpGrid OPM/opm-simulators#5633

Closed

aritorto force-pushed the globalCellLgr branch from f1f25d0 to c70338f Compare September 27, 2024 10:06

aritorto mentioned this pull request Sep 27, 2024

Local Cartesian Index for level grids #765

Closed

aritorto changed the title ~~Local Cartesian Indices for LGRs~~ Level global cell for CpGrid with LGRs Sep 27, 2024

blattms reviewed Sep 27, 2024

View reviewed changes

aritorto added 4 commits September 30, 2024 08:06

Modify global_cell_ for lgrs

d79e1ce

Remove empty line

d342444

Refactor and separate inParentCellIJK computation

75742a4

Remove spaces

d54de16

Remove nonneeded copy, add cells_per_dim as funct arg

1dda48a

aritorto force-pushed the globalCellLgr branch from c70338f to a8aeded Compare September 30, 2024 10:00

aritorto added 2 commits September 30, 2024 12:45

Chage size(0) for logicalCartesianSize() multiplication

d0c7131

Check global cell max value below maxCartesianIdx

de130b7

aritorto force-pushed the globalCellLgr branch from a8aeded to de130b7 Compare September 30, 2024 11:03

blattms reviewed Sep 30, 2024

View reviewed changes

Keep only one getIJK

266320c

blattms merged commit 057de17 into OPM:master Oct 1, 2024
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Level global cell for CpGrid with LGRs #762

Level global cell for CpGrid with LGRs #762

aritorto commented Sep 25, 2024 •

edited

Loading

aritorto commented Sep 25, 2024

aritorto commented Sep 25, 2024

aritorto commented Sep 26, 2024

aritorto commented Sep 26, 2024

blattms commented Sep 27, 2024

blattms left a comment

blattms Sep 27, 2024 •

edited

Loading

aritorto Sep 30, 2024

blattms Sep 27, 2024

aritorto Sep 30, 2024

blattms Sep 27, 2024

aritorto Sep 30, 2024

aritorto commented Sep 30, 2024 •

edited

Loading

blattms commented Sep 30, 2024

aritorto commented Sep 30, 2024

aritorto commented Sep 30, 2024

aritorto commented Sep 30, 2024

blattms Sep 30, 2024

aritorto Sep 30, 2024

aritorto Sep 30, 2024

aritorto commented Oct 1, 2024

Level global cell for CpGrid with LGRs #762

Level global cell for CpGrid with LGRs #762

Conversation

aritorto commented Sep 25, 2024 • edited Loading

aritorto commented Sep 25, 2024

aritorto commented Sep 25, 2024

aritorto commented Sep 26, 2024

aritorto commented Sep 26, 2024

blattms commented Sep 27, 2024

blattms left a comment

Choose a reason for hiding this comment

blattms Sep 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aritorto commented Sep 30, 2024 • edited Loading

blattms commented Sep 30, 2024

aritorto commented Sep 30, 2024

aritorto commented Sep 30, 2024

aritorto commented Sep 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aritorto commented Oct 1, 2024

aritorto commented Sep 25, 2024 •

edited

Loading

blattms Sep 27, 2024 •

edited

Loading

aritorto commented Sep 30, 2024 •

edited

Loading