Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AMR for non-cell centered fields #904

Merged
merged 144 commits into from
Aug 18, 2023
Merged

AMR for non-cell centered fields #904

merged 144 commits into from
Aug 18, 2023

Conversation

lroberts36
Copy link
Collaborator

@lroberts36 lroberts36 commented Jul 11, 2023

PR Summary

This PR does the last few things required to make full AMR work with non-cell centered fields:

  • Separates the prolongation and restriction caches from the boundary caches by creating a new ProResInfo type that is similar to BndInfo.
  • Unifies the boundary index calculation routine further.
  • Switch to restriction in one during RedistributeAndRefineMeshBlocks
  • Switch to prolongation in one during RedistributeAndRefineMeshBlocks
  • Updates TryRecvFineToCoarse to deal with non-cell centered fields
  • Updates TryRecvCoarseToFine to deal with non-cell centered fields
  • Test that things actually work
  • Come up with some good test problems for Parthenon (any ideas are appreciated)

B^2/2 w/ AMR
Screenshot 2023-07-11 at 2 27 01 PM

divB w/ AMR
Screenshot 2023-07-11 at 2 26 45 PM

PR Checklist

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

@lroberts36 lroberts36 changed the base branch from lroberts36/add-index-range-masking to develop July 18, 2023 18:56
@lroberts36
Copy link
Collaborator Author

lroberts36 commented Jul 27, 2023

A possible test problem is to evolve

$$\partial_t \vec{F} + \nabla \times \vec{G} = 0$$ $$\partial_t \vec{G} + \nabla \times \vec{F} = 0$$

which should obey the wave equation but also have $\nabla \cdot \vec{F} = 0$ and $\nabla \cdot \vec{G} = 0$.

Copy link
Collaborator

@Yurlungur Yurlungur 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 sorry it took me so long to review this... looks like I had started reviewing it and then I must have gotten interrupted and forgotten because I never finished. Overall this looks good.

One thing: we should probably document the ownership model somewhere in the docs.

src/mesh/mesh_refinement.cpp Show resolved Hide resolved
src/mesh/logical_location.hpp Show resolved Hide resolved
src/bvals/comms/bnd_info.hpp Show resolved Hide resolved
src/bvals/comms/bnd_info.hpp Show resolved Hide resolved
src/bvals/comms/bnd_info.cpp Show resolved Hide resolved
@Yurlungur
Copy link
Collaborator

P.S. I am so excited by this

@Yurlungur
Copy link
Collaborator

A possible test problem is to evolve

which should obey the wave equation but also have ∇⋅F→=0 and ∇⋅G→=0.

👍 Vector wave equation seems like a good test problem and a nice demonstration of the technology. Maybe as a new example?

@lroberts36
Copy link
Collaborator Author

lroberts36 commented Jul 27, 2023

Yeah, we should add it as an example. I think maybe we save that for a documentation/test PR. As you say, it would be good to have documentation about how the ownership model works, etc. It would be especially good to get done before it is forgotten...

@Yurlungur
Copy link
Collaborator

Yeah I think the ownership model makes sense, and your code is clear. But it's sufficiently different from how Athena++ does it, and there are a sufficiently large number of choices to make, we should clarify which choices we made in a single place.

@Yurlungur
Copy link
Collaborator

I'm fine with putting docs and an example off into a final PR though.

@lroberts36 lroberts36 mentioned this pull request Aug 3, 2023
8 tasks
Copy link
Collaborator

@pdmullen pdmullen left a comment

Choose a reason for hiding this comment

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

Not too much to contribute here... LGTM. The test problems you have presented from various downstream applications give me confidence in the implementation.

src/defs.hpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@bprather bprather left a comment

Choose a reason for hiding this comment

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

Finally using this in earnest and it works great. Can't yet provide a working downstream due to my own bugs, but the machinery's all there and the interface has been easy to use.
Though it shouldn't delay this PR longer, we should remember to add the InternalRefinementOp template arg to Metadata when we get a chance, I started implementing the B field internal refinement outside of Parthenon proper and was briefly stung.

@bprather bprather merged commit 0830cde into develop Aug 18, 2023
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plan for face-centered fields
6 participants