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

Reflecting boundary conditions #58

Closed
wants to merge 5 commits into from
Closed

Reflecting boundary conditions #58

wants to merge 5 commits into from

Conversation

BenWibking
Copy link
Contributor

@BenWibking BenWibking commented May 4, 2023

Partially fixes #56.

Only X3-boundaries are implemented. The boundary functions must be manually enrolled by the problem generator.

@BenWibking BenWibking requested a review from pgrete May 4, 2023 16:19
Copy link
Contributor

@pgrete pgrete 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 porting this over.
In addition to the inline comments I got two more:
a) do we have an "easy" way to test those boundary conditions?
b) some doc on how to enroll those (versus the ones in Parthenon) might be useful.

Comment on lines +22 to +53
// Function for checking boundary flags: is this a domain or internal bound?
//
inline bool IsDomainBound(parthenon::MeshBlock *pmb, parthenon::BoundaryFace face) {
return !(pmb->boundary_flag[face] == parthenon::BoundaryFlag::block ||
pmb->boundary_flag[face] == parthenon::BoundaryFlag::periodic);
}

// Get zones which are inside the physical domain, i.e. set by computation or MPI halo
// sync, not by problem boundary conditions.
//
inline auto GetPhysicalZones(parthenon::MeshBlock *pmb, parthenon::IndexShape &bounds)
-> std::tuple<parthenon::IndexRange, parthenon::IndexRange, parthenon::IndexRange> {
return std::tuple<parthenon::IndexRange, parthenon::IndexRange, parthenon::IndexRange>{
parthenon::IndexRange{IsDomainBound(pmb, parthenon::BoundaryFace::inner_x1)
? bounds.is(parthenon::IndexDomain::interior)
: bounds.is(parthenon::IndexDomain::entire),
IsDomainBound(pmb, parthenon::BoundaryFace::outer_x1)
? bounds.ie(parthenon::IndexDomain::interior)
: bounds.ie(parthenon::IndexDomain::entire)},
parthenon::IndexRange{IsDomainBound(pmb, parthenon::BoundaryFace::inner_x2)
? bounds.js(parthenon::IndexDomain::interior)
: bounds.js(parthenon::IndexDomain::entire),
IsDomainBound(pmb, parthenon::BoundaryFace::outer_x2)
? bounds.je(parthenon::IndexDomain::interior)
: bounds.je(parthenon::IndexDomain::entire)},
parthenon::IndexRange{IsDomainBound(pmb, parthenon::BoundaryFace::inner_x3)
? bounds.ks(parthenon::IndexDomain::interior)
: bounds.ks(parthenon::IndexDomain::entire),
IsDomainBound(pmb, parthenon::BoundaryFace::outer_x3)
? bounds.ke(parthenon::IndexDomain::interior)
: bounds.ke(parthenon::IndexDomain::entire)}};
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this code actually used somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is used in my precipitator pgen. I could move it there if you don't want to include it here.


using parthenon::Real;

void ReflectingInnerX3(std::shared_ptr<parthenon::MeshBlockData<Real>> &mbd,
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to not template this (for different dims) already now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can do that.

Comment on lines +87 to +89
std::string label = (TYPE == BCType::Reflect ? "Reflect" : "Outflow");
label += (INNER ? "Inner" : "Outer");
label += "X" + std::to_string(DIR);
Copy link
Contributor

Choose a reason for hiding this comment

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

No need for outflow here but maybe add "AthenaPK" to the label.

label, nvar, domain, coarse,
KOKKOS_LAMBDA(const int &l, const int &k, const int &j, const int &i) {
if (!q.IsAllocated(l)) return;
if (TYPE == BCType::Reflect) {
Copy link
Contributor

Choose a reason for hiding this comment

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

We can probably skip this TYPE and just call this method `ApplyReflectBC``

@BenWibking
Copy link
Contributor Author

For tests, are you thinking of a simulation that would run and then we would check the solution, or just a unit test to check that the ghost cells are filled correctly for a given example of interior cells?

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.

fix reflecting boundary conditions
2 participants