Bug Fix: Boundary prolongation only occurring on base stage

[lroberts36]

PR Summary

This PR fixes a bug in the TaskStatus ProlongateBoundaries(std::shared_ptr<MeshBlockData<Real>> &rc) logic that caused prolongation to occur on the base stage MeshBlockData container of a given block, independent of which MeshBlockData stage was passed to ProlongateBoundaries. Because BoundaryValues::ProlongateBoundaries() was eventually called from the BoundaryValues object owned by the parent MeshBlock of the MeshBlockData, no information was passed through about which MeshBlockData associated with a given MeshBlock was asking for the boundary prolongation to occur. This PR also refactors the boundary prolongation code a bit so that there is not a hard to follow chain of calls (which is probably why this bug has persisted).

This fix of course causes regression tests that include mesh refinement to fail. If I switch the line

boundary_cond_impl::ProlongateGhostCells_(rc, nb, bi.s, bi.e, bj.s, bj.e, bk.s, bk.e);

to

boundary_cond_impl::ProlongateGhostCells_(pmb->meshblock_data.Get(), 
    nb, bi.s, bi.e, bj.s, bj.e, bk.s, bk.e);

in parthenon::ProlongateBoundaries(std::shared_ptr<MeshBlockData<Real>> &rc) all of the regression tests pass on v13. I have added a new set of updated gold files as v15 with the bug fixed.

Thanks to @pdmullen for finding that some ghost zone values were unset during intermediate RK stages which pointed to this issue.

PR Checklist

• Code passes cpplint
• Adds a test for any bugs fixed. Adds tests for new features.
• Code is formatted
• Changes are summarized in CHANGELOG.md
• CI has been triggered on Darwin for performance regression tests.
• (@lanl.gov employees) Update copyright on changed files

[pdmullen]

Looks good to me! I can confirm that @lroberts36's proposed fix solves issues I was encountering with ghost cell filling with mesh refinement.

The downstream codes I am working with point to here: f7f4e17 , but as I hope to be evidenced by regression testing, I trust this iteration of the fix equally addresses the issue.

[lroberts36]

@pdmullen: I will change the fix on the Riot parthenon branch to be the same as this just to double check.

[pdmullen]

@pdmullen: I will change the fix on the Riot parthenon branch to be the same as this just to double check.

Sounds great; I'll keep an eye out for it. I'll rerun my tests and report back here.

[pdmullen]

@lroberts36 mimicked the changes in this PR in the lroberts36/merge-sparse-with-jdolence-sparse branch which our downstream code points to. I can confirm that

boundary_cond_impl::ProlongateGhostCells_(rc, nb, bi.s, bi.e, bj.s, bj.e, bk.s, bk.e);

vs

boundary_cond_impl::ProlongateGhostCells_(pmb->meshblock_data.Get(), 
    nb, bi.s, bi.e, bj.s, bj.e, bk.s, bk.e);

makes the difference between a passing and failing test, respectively. From my perspective, this PR is good to go.

[bprather]

If Patrick vouches that it fixes the issue, I for one am happy with how it does. Always love a net-negative line count.

[Yurlungur]

Nice catch! I wish I'd caught this when I was mucking about in prolongation/restriction.

To update the gold files, run the tests locally, then zip up the files and make a new release. Add the zip files to the release, and update the version and hash in the cmake.

[Yurlungur]

To update the gold files, run the tests locally, then zip up the files and make a new release. Add the zip files to the release, and update the version and hash in the cmake.

Maybe @pgrete has some magic to automate this.

[lroberts36]

I made the release that updates the gold files for this bug fix a pre-release. Once this is merged, I guess it should be switched to being the latest release.

[pgrete]

To update the gold files, run the tests locally, then zip up the files and make a new release. Add the zip files to the release, and update the version and hash in the cmake.

Maybe @pgrete has some magic to automate this.

No magic here but that reminds me that we should probably document those steps (and also how to make a release) -- opened an issue for this #823 .

[pgrete]

LGTM I just have one cleanup suggestion and one clarification request (both non-blocking)