Skip to content

Fix shiftedmetricinterp unit test#3277

Closed
ZedThree wants to merge 3 commits into
nextfrom
fix-shiftedmetricinterp-unit-test
Closed

Fix shiftedmetricinterp unit test#3277
ZedThree wants to merge 3 commits into
nextfrom
fix-shiftedmetricinterp-unit-test

Conversation

@ZedThree
Copy link
Copy Markdown
Member

I'd forgotten to commit setting GlobalNzNoBoundaries in FakeMesh 🤦

As discussed in the other PR, I've also set the region for Z interpolation to be NOY

@ZedThree ZedThree requested a review from dschwoerer February 17, 2026 15:35
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread src/mesh/interpolation/interpolation_z.cxx
Comment thread src/mesh/interpolation/interpolation_z.cxx
Comment thread src/mesh/interpolation/interpolation_z.cxx
Copy link
Copy Markdown
Contributor

@dschwoerer dschwoerer left a comment

Choose a reason for hiding this comment

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

Thanks 👍

@ZedThree
Copy link
Copy Markdown
Member Author

Ah, we have found the test fails when the region manipulation in ZInterpolation isn't done -- test-laplace-petsc3d with CHECK=4 sets the boundaries to NaN, and ShiftedMetricInterp ends up hitting these.

This is coming from communicating the solution inside the Laplace solver. The solver is only applying its BCs one boundary cell deep, and we get a NaN from the second one (the outer most boundary cell). We (probably) can't just apply our own BCs here in case they conflict with the solver's, but it's possible we could just extrapolate or something to the outer boundary cells.

I think the simplest thing is to just revert those changes.

@ZedThree ZedThree closed this Feb 20, 2026
@ZedThree ZedThree deleted the fix-shiftedmetricinterp-unit-test branch February 20, 2026 09:22
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.

2 participants