Skip to content

Conversation

@arturcastiel
Copy link
Member

This PR is a continuation of the work at 4839. It performs generates the ZCORN for any general CPG.

@arturcastiel
Copy link
Member Author

jenkins build this please

@akva2 akva2 added the manual:irrelevant This PR is a minor fix and should not appear in the manual label Dec 1, 2025
@arturcastiel
Copy link
Member Author

@akva2 thank you for adding the manual tag.

@arturcastiel
Copy link
Member Author

jenkins build this please

@arturcastiel
Copy link
Member Author

@bska

@blattms
Copy link
Member

blattms commented Dec 16, 2025

I have one question about this:

During output opm-simulators will create a copy of the EclipseGrid with overridden zcorn using EclipseGrid(const EclipseGrid& src, const double* zcorn, const std::vector<int>& actnum) (triggered from opm/grid/cpgrid/GridHelpers.cpp#L55

Eventually this copy constructor will need to take zcorn for all LGR grids.
Currently these are only the ones for the global grid. What effect will the call have with this PR? Will it correctly modify zcorn for the global grid?

If it is not there, yet, then we should probably add a test using that copy constructor.

… on COORD and ZCORN of parent grid only

added maybe unused to not implemented zcorn routine

ZCORN refinement routines added

removing set_refinement as the refinement is automatic now
@arturcastiel
Copy link
Member Author

jenkins build this please

@arturcastiel
Copy link
Member Author

@bska

Copy link
Member

@bska bska left a comment

Choose a reason for hiding this comment

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

I'm having trouble recognising every part here so I think we'll at least need some more documentation on what each helper function does. Ideally, also an overall/high level description of the strategy.

Finally, I'm probably misreading the algorithm, but I don't quite see where it handles cells of varying thicknesses and especially cells that aren't completely hexagonal hexahedral.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I understand this algorithm, so I think I'd like some additional explanation. Does it do uniform subdivision in the vertical direction between the top and bottom pillar points? If so, we're going to run into issues if the cells don't have uniform thickness.

As an example, consider the (academic) setup

DIMENS
 1 1 3 /

COORD
  0 0 0  0 0 0
  1 0 0  1 0 0
  0 1 0  0 1 0
  1 1 0  1 1 0
/

ZCORN
  0 0
  0 0
  3 3
  3 3

  3 3
  3 3
  9 9
  9 9

  9 9
  9 9
  18 18
  18 18
/

which is a 1-by-1-by-3 cell geometry in which the cells have physical dimensions

  • Cell 1,1,1: $1 \times 1\times 3$ metres
  • Cell 1,1,2: $1 \times 1\times 6$ metres
  • Cell 1,1,3: $1 \times 1\times 9$ metres

If we then define

CARFIN
  LGR1  1 1  1 1  1 3   1 1 9 /
ENDFIN

then main grid cell 1,1,1 should be subdivided into three cells of dimension $1\times 1\times 1$ metres, main grid cell 1,1,2 should be subdivided into three cells of dimension $1\times 1\times 2$ metres, and main grid cell 1,1,3 should be subdivided into three cells of dimension $1\times 1\times 3$ metres.

Does your algorithm do that? If so, please add this case as another unit test.

@arturcastiel
Copy link
Member Author

jenkins build this please

@arturcastiel
Copy link
Member Author

@bska based on our discussions, there is a follow up test.

Copy link
Member

@bska bska left a comment

Choose a reason for hiding this comment

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

based on our discussions, there is a follow up test.

Okay, I'll merge this, but I really would have liked one or more small unit tests that don't read or write files when we're testing this. My column example from before, for instance, does not need any of that and is a lot easier to analyse in isolation. The tests here are more like system integration tests that depend on a lot of components working together.

@bska bska merged commit 16953de into OPM:master Dec 18, 2025
2 checks passed
@arturcastiel arturcastiel deleted the lgr-zcorn branch December 19, 2025 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual:irrelevant This PR is a minor fix and should not appear in the manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants