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

Add more runtime NGHOST checks (especially for SMR/AMR) #163

Closed
felker opened this issue Oct 5, 2018 · 0 comments
Closed

Add more runtime NGHOST checks (especially for SMR/AMR) #163

felker opened this issue Oct 5, 2018 · 0 comments
Assignees
Labels
bug Broken functionality or unexpected result documentation Relating to the Wiki, Website, or Markdown files good first issue Ideal for developers looking to dive into the Athena++ codebase SMR/AMR Relating to static and adaptive mesh refinement
Milestone

Comments

@felker
Copy link
Contributor

felker commented Oct 5, 2018

As I was adding a myriad of runtime checks to reconstruction.cpp for ensuring compatibility with the 4th order solver in #157, it occurred to me that there are few traps on the selected NGHOST value because it was assumed that NGHOST=2 for most of the history of Athena++. NGHOST was generalized to a compile-time parameter and configure.py flag --nghost in 5721084 on 2018-03-03 in order to support PPM and other high-order solver components.

Here are the following cases I have encountered or envisioned that could benefit from a safety check:

  • For periodic boundary conditions,block_size.nx1 >= NGHOST/2 in all MeshBlocks regardless of the algorithm.
    • This has become an issue when using NGHOST=4 for PPM with MHD with small MeshBlocks with meshblock/nx2=6, for example.
  • NGHOST % 2 == 0 when using SMR/AMR.
    • I was using NGHOST=3 minimum requirement for PPM for R-T instability simulations with AMR in Fix asymmetry of Coordinates x1f for multiple MeshBlock grids #112, but I believe that should have been forbidden / rounded up to NGHOST=4.
    • Actually, the restriction is stronger insofar that the ghost cell buffer size should be a power of 2 on all sides, right @tomidakn?
    • The Wiki documentation for SMR and AMR predate the NGHOST>2 change, so they should probably be updated.

In the interest of protecting the users from themselves, I think we should add these conditions to the Reconstruction class constructor (or somewhere else) and document the requirements in the Wiki.

@felker felker added bug Broken functionality or unexpected result SMR/AMR Relating to static and adaptive mesh refinement documentation Relating to the Wiki, Website, or Markdown files labels Oct 5, 2018
@felker felker self-assigned this Oct 5, 2018
@felker felker added this to Low priority in Bugs Oct 9, 2018
@felker felker added this to the v1.2.0 milestone Oct 9, 2018
@felker felker added the good first issue Ideal for developers looking to dive into the Athena++ codebase label Jan 22, 2019
@felker felker closed this as completed Jan 20, 2021
Usability, tools, outputs automation moved this from To do to Done Jan 20, 2021
Bugs automation moved this from Low priority to Closed Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Broken functionality or unexpected result documentation Relating to the Wiki, Website, or Markdown files good first issue Ideal for developers looking to dive into the Athena++ codebase SMR/AMR Relating to static and adaptive mesh refinement
Projects
Bugs
  
Closed
Development

No branches or pull requests

1 participant