Skip to content

[LGR] Fix wrong parent intersection lookup at LGR coarse/fine interfaces#1022

Merged
aritorto merged 1 commit intoOPM:masterfrom
arturcastiel:lgr-bugfix
Apr 17, 2026
Merged

[LGR] Fix wrong parent intersection lookup at LGR coarse/fine interfaces#1022
aritorto merged 1 commit intoOPM:masterfrom
arturcastiel:lgr-bugfix

Conversation

@arturcastiel
Copy link
Copy Markdown
Member

@arturcastiel arturcastiel commented Apr 10, 2026

Fix

Ensure getParentIntersectionFromLgrBoundaryFace always returns the correct level-0 (parent/coarse) intersection at LGR coarse/fine interfaces, independent of whether the refined cell is intersection.inside() or intersection.outside().

The implementation now maps the leaf-grid intersection to the level-0 grid using getOrigin() and performs comparisons using indexInInside() within the same grid level. This avoids mismatches caused by using indices from different cells with opposite local face orientations.

This guarantees:

  • Correct parent intersection selection for both inside/outside refined configurations.
  • Consistent face index comparisons within the level-0 grid.

Impact

Restores correct geometric references at LGR boundaries and ensures symmetric and physically consistent transmissibility calculations across coarse/fine interfaces.

@arturcastiel arturcastiel added the manual:bugfix This PR is a bug fix and should be noted in the manual label Apr 10, 2026
@arturcastiel
Copy link
Copy Markdown
Member Author

jenkins build this please

@arturcastiel arturcastiel requested a review from blattms April 13, 2026 08:08
@aritorto
Copy link
Copy Markdown
Member

aritorto commented Apr 13, 2026

This looks like a great catch. I would suggest slightly revising the PR description, for example:

"[...] intersection.indexInInside() is the local face index in the intersection.inside() cell, which may correspond to either the refined cell or the coarse cell. In the latter case, the comparison with parentIntersection.indexInInside()—the local face index in the parent cell—yields an incorrect conclusion. [...]"

@arturcastiel
Copy link
Copy Markdown
Member Author

@aritorto thank you for the suggestions. I will look them carefully.

@aritorto
Copy link
Copy Markdown
Member

aritorto commented Apr 13, 2026

@arturcastiel, I added a test that should fail based on these findings, but it currently passes. I’ll look into it and follow up!

One thing I forgot to mention is that this method is intended to handle intersections belonging to both coarse and refined cells, since both appear as cells in the leaf grid view. In both cases, it should be possible to return the corresponding original coarse intersection, regardless of whether the given intersection belongs to a coarse or a refined cell, as shown in the test in #1023

@arturcastiel
Copy link
Copy Markdown
Member Author

@aritorto should we talk about this in a quick meeting?

Copy link
Copy Markdown
Member

@aritorto aritorto left a comment

Choose a reason for hiding this comment

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

This can be simplified by using getOrigin(), which returns either:

the coarsest ancestor in the level-0 grid if intersection.inside() is refined, or
the geometrically equivalent cell if it is a leaf cell that was never refined.

This avoids explicitly checking whether the cell is coarse or refined, and ensures that the comparison between face tag and orientation remains consistent

Copy link
Copy Markdown
Member

@aritorto aritorto left a comment

Choose a reason for hiding this comment

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

I make the suggestion even though it's still in draft mode

Comment thread opm/grid/cpgrid/CpGrid.cpp Outdated
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.

Suggested change
// Skip case when both cells belong to different refined level grids
if (intersection.inside().level()*intersection.outside().level() == 0) {
// Get the equivalent level-0 cell if intersection.inside().level() == 0;
// otherwise, get the coarsest ancestor at level 0.
const auto insideOrigin = intersection.inside().getOrigin();
for(const auto& originIntersection : intersections(this->levelGridView(0), insideOrigin)){
if (originIntersection.indexInInside() == intersection.indexInInside()) {
return originIntersection;
}
}
}

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.

I couldn’t include all the lines I’d remove in this suggestion, but I hope the intended change is still clear. Let me know if anything is unclear—I’m happy to clarify!

@arturcastiel
Copy link
Copy Markdown
Member Author

jenkins build this please

Copy link
Copy Markdown
Member

@aritorto aritorto 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 incorporating the suggestion. We still need to include the line that verifies that the cells attached to the given intersection come from different grid levels, in addition to checking that one of those levels is zero.

@arturcastiel
Copy link
Copy Markdown
Member Author

true, hold on

Dune::cpgrid::Intersection CpGrid::getParentIntersectionFromLgrBoundaryFace(const Dune::cpgrid::Intersection& intersection) const
{
if ( intersection.neighbor()) {
if ((intersection.inside().level() != intersection.outside().level())) {
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 also need this line, or somehow check both:

  • inside and outside elements belong to different level grids
  • one of those levels must be zero (we do not handle the case when cells attached to the intersection belong to two different refined level grids)

Use intersection.inside().getOrigin() to obtain the level-0 ancestor of
the inside cell, then match on intersection.indexInInside() directly:

- If inside is a coarse (level-0) cell, getOrigin() returns the same cell
  and indexInInside() is already the correct level-0 face index.
- If inside is a refined cell, getOrigin() returns its level-0 parent and
  indexInInside() equals the parent face index (fine cells inherit face
  directions from their parent).

The guard `inside().level() * outside().level() == 0` skips the unhandled
case where both cells are from different non-zero refinement levels.
@arturcastiel
Copy link
Copy Markdown
Member Author

jenkins build this please

@aritorto
Copy link
Copy Markdown
Member

aritorto commented Apr 16, 2026

@arturcastiel would you mind updating the “Problem” and “Fix” sections in the PR description as well?

Key points:

  • The method returns the parent (coarse) intersection, regardless of whether intersection.inside() refers to a coarse or refined cell.
  • The comparison using intersection.indexInInside() was incorrect in cases where the refined cell corresponds to intersection.outside().
  • The new approach, using getOrigin(), ensures a correct comparison of indexInInside between leaf-grid and level-zero-grid intersections.

Also, is there a reason the PR is still in draft mode?

@arturcastiel arturcastiel marked this pull request as ready for review April 16, 2026 12:18
@aritorto aritorto self-requested a review April 16, 2026 13:09
@aritorto
Copy link
Copy Markdown
Member

jenkins build this serial please

@aritorto
Copy link
Copy Markdown
Member

@arturcastiel thanks for incorporating the suggestion and updating the PR description, merging!

@aritorto aritorto merged commit dbba62f into OPM:master Apr 17, 2026
2 checks passed
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.

2 participants