Skip to content

Skip NNCs created by PINCH if one of the cells is collapsed.#935

Open
blattms wants to merge 2 commits into
OPM:masterfrom
blattms:bugfix/PINCH-NNC-with-collapsed-cells
Open

Skip NNCs created by PINCH if one of the cells is collapsed.#935
blattms wants to merge 2 commits into
OPM:masterfrom
blattms:bugfix/PINCH-NNC-with-collapsed-cells

Conversation

@blattms

@blattms blattms commented Oct 14, 2025

Copy link
Copy Markdown
Member

Collapsed here means that the points of the top and lower plane on the same pillar coincide.
Later those cells will be treated as inactive in preprocess.c and will not be part of the grid. when we create the grid topology. They would present numerical problems later anyway.

Note that this means that these cells (collapsed but PORV larger than MINPVV) will now be present a barrier preventing creation of a PINCH NNC.

Fixes the assertion:

flow: ./opm/grid/cpgrid/EntityRep.hpp:121: void Dune::cpgrid::EntityRep<codim>::setValue(int, bool) [with int codim = 0]: Assertion `index_arg >= 0' failed.

Closes #505

@blattms blattms added this to the Release 2025.10 milestone Oct 14, 2025
@blattms blattms added the manual:bugfix This PR is a bug fix and should be noted in the manual label Oct 14, 2025
@blattms

blattms commented Oct 14, 2025

Copy link
Copy Markdown
Member Author

jenkins build this please

@blattms

blattms commented Oct 15, 2025

Copy link
Copy Markdown
Member Author

With this the collapsed cell will prevent creation of an NNC. It might turn out that this is not what we want, but it is certainly way better than a model aborting in an assert. Hence I think this should be merged but we should compare behavior with others.

@bska

bska commented Oct 21, 2025

Copy link
Copy Markdown
Member

I'd really like to see some unit tests for this in order to assess the behaviour.

@daavid00

Copy link
Copy Markdown
Member

Hei, just a frienldy reminder about the schedule for the release:

Bug fixing deadline for RC2: Thursday the 23rd of October, 6pm CET.

If we are backporting this, then we need to make a RC3.

@blattms

blattms commented Oct 24, 2025

Copy link
Copy Markdown
Member Author

jenkins build this please

1 similar comment
@blattms

blattms commented Dec 30, 2025

Copy link
Copy Markdown
Member Author

jenkins build this please

blattms added 2 commits April 15, 2026 10:43
Collaped here means that the points of the top and lower plane all
coincide.
Later those cells will be treated as inactive in preprocess.c when
create the grid topology. They qould presemt numerical problems later
anyway.

Note that this means that these cells (collapsed but PORV larger than
MINPVV) will now be present a barrier for PINCH.

Fixes the assertion:
flow: ./opm/grid/cpgrid/EntityRep.hpp:121: void Dune::cpgrid::EntityRep<codim>::setValue(int, bool) [with int codim = 0]: Assertion `index_arg >= 0' failed.
@blattms

blattms commented Apr 15, 2026

Copy link
Copy Markdown
Member Author

Bugfix from October. Maybe this can make it into the release?

There is a test added in case you were wondering.

@blattms blattms force-pushed the bugfix/PINCH-NNC-with-collapsed-cells branch from 0f2cfa0 to 188bec4 Compare April 15, 2026 08:50
@blattms

blattms commented Apr 15, 2026

Copy link
Copy Markdown
Member Author

jenkins build this please

@plgbrts

plgbrts commented Jun 24, 2026

Copy link
Copy Markdown

Thanks @blattms for this fix. With it, a big field case (18M cells) was able to run that otherwise gives the mentioned assert. It would be great if this PR can be merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual:bugfix This PR is a bug fix and should be noted in the manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BuildFaceToCell Calling EntityRep::setValue() With Index of "-1"

4 participants