-
Notifications
You must be signed in to change notification settings - Fork 348
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
Bittree updates #2893
Bittree updates #2893
Conversation
Currently performing tests. This PR is created to enable discussion as we tweak the code and get it ready for production. |
This is done. Also created a separate directories |
|
Additional task: Setup a test in AMReX test suite @WeiqunZhang can you do that on your end? |
@@ -31,15 +31,17 @@ amr.max_level = 2 # maximum level number allowed -- | |||
|
|||
amr.ref_ratio = 2 2 2 2 # refinement ratio between levels | |||
|
|||
amr.use_bittree = true | |||
|
|||
# ***************************************************************** | |||
# Control of grid creation | |||
# ***************************************************************** | |||
# Blocking factor for grid creation in each dimension -- | |||
# this ensures that every grid is coarsenable by a factor of 8 -- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the comment ("every grid is coarsenable by a factor of 8") need updating?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. @akashdhruv and @tklos96 Do we still need to set blocking factors and max_grid_size in inputs_bittree
?
Yes, once the bittree repo is public, I will set up a regression test. |
@@ -260,6 +264,11 @@ protected: | |||
Vector<DistributionMapping> dmap; | |||
Vector<BoxArray> grids; | |||
|
|||
#ifdef AMREX_USE_BITTREE | |||
bool use_bittree; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should move this flag into AmrInfo
and gives it a default value of false. One constructor of AmrMesh does not use ParmParse at all and it relies on the parameters in AmrInfo
. Then for consistence, in InitAmrMesh
, we should change from get
to queryAdd
.
The last time I tried, I could not reproduce the blow-up issue with subcycling. After the two minor comments are addressed, I think we can merge this first, because this is a big PR. |
I found an issue with a 3D setup in Flash-X. Give me a few days to resolve that and then we can merge |
Continued in #3547 |
…mode (#3555) ## Summary This PR introduces dependency on [Bittree library](https://github.com/Flash-X/Bittree) to improve regridding performance in octree mode. ## Additional background Testing and development of this feature is done in sync with Flash-X and is recorded in this [reproducibility capsule](https://github.com/Lab-Notebooks/AMReX-Bittree-Performance). This PR is primarily created to consolidate development work and avoid creating multiple branches. At present using AMReX+Bittree improves regridding performance [by a factor of 2](https://github.com/Lab-Notebooks/AMReX-Bittree-Performance/blob/14faa2212c4e5dba7fd99a6526c6937414f9c109/analysis/Performance.ipynb) at > 20000 ranks. We hope to improve performance further using a bittree-based distribution mapping and therefore adding a new function `AmrMesh::MakeDistributionMap` as a place-holder. Continuation of #2893 and #3547 ## Checklist The proposed changes: - [ ] fix a bug or incorrect behavior in AMReX - [x] add new capabilities to AMReX - [ ] changes answers in the test suite to more than roundoff level - [ ] are likely to significantly affect the results of downstream AMReX users - [ ] include documentation in the code and/or rst files, if appropriate --------- Co-authored-by: Weiqun Zhang <[email protected]> Co-authored-by: Tom Klosterman <https://github.com/tklos96>
…mode (AMReX-Codes#3555) ## Summary This PR introduces dependency on [Bittree library](https://github.com/Flash-X/Bittree) to improve regridding performance in octree mode. ## Additional background Testing and development of this feature is done in sync with Flash-X and is recorded in this [reproducibility capsule](https://github.com/Lab-Notebooks/AMReX-Bittree-Performance). This PR is primarily created to consolidate development work and avoid creating multiple branches. At present using AMReX+Bittree improves regridding performance [by a factor of 2](https://github.com/Lab-Notebooks/AMReX-Bittree-Performance/blob/14faa2212c4e5dba7fd99a6526c6937414f9c109/analysis/Performance.ipynb) at > 20000 ranks. We hope to improve performance further using a bittree-based distribution mapping and therefore adding a new function `AmrMesh::MakeDistributionMap` as a place-holder. Continuation of AMReX-Codes#2893 and AMReX-Codes#3547 ## Checklist The proposed changes: - [ ] fix a bug or incorrect behavior in AMReX - [x] add new capabilities to AMReX - [ ] changes answers in the test suite to more than roundoff level - [ ] are likely to significantly affect the results of downstream AMReX users - [ ] include documentation in the code and/or rst files, if appropriate --------- Co-authored-by: Weiqun Zhang <[email protected]> Co-authored-by: Tom Klosterman <https://github.com/tklos96>
Summary
This PR introduces updates to regridding using Bittree
Additional background
We have introduced a new feature to use AMR in Octree mode. This was a result of ongoing performance studies which identified that Octree mode using Bittree scales better than AMReX native patch-based implementation.
Checklist
The proposed changes: