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

Bittree updates #2893

Closed
wants to merge 51 commits into from
Closed

Bittree updates #2893

wants to merge 51 commits into from

Conversation

akashdhruv
Copy link
Contributor

@akashdhruv akashdhruv commented Jul 29, 2022

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:

  • fix a bug or incorrect behavior in AMReX
  • 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

@akashdhruv
Copy link
Contributor Author

Currently performing tests. This PR is created to enable discussion as we tweak the code and get it ready for production.

@akashdhruv
Copy link
Contributor Author

We need to introduce USE_BITTREE into GNUMake system and add -DAMREX_USE_BITTREE to the compiler flags. Then we can use ifdef AMREX_USE_BITTREE on the bittree code that has extra dependence.

This is done. Also created a separate directories Src/Extern/Bittree and Tools/GNUmake/packages/Make.bittree to manage source code and dependencies.

@akashdhruv
Copy link
Contributor Author

akashdhruv commented Aug 24, 2022

./configure --enable-bittree=yes is the new option to build AMReX in Bittree mode. Default is no. Environment variable BITTREE_$(DIM)D_HOME should be defined to store Bittree library path. $(DIM) is the dimension

@akashdhruv akashdhruv changed the title [WIP] Bittree updates Bittree updates Aug 24, 2022
@akashdhruv
Copy link
Contributor Author

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 --
Copy link
Contributor

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?

Copy link
Member

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?

@WeiqunZhang
Copy link
Member

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;
Copy link
Member

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.

@WeiqunZhang
Copy link
Member

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.

@akashdhruv
Copy link
Contributor Author

I found an issue with a 3D setup in Flash-X. Give me a few days to resolve that and then we can merge

@akashdhruv
Copy link
Contributor Author

Performance of Paramesh vs AMReX in Flash-X

bittree

@akashdhruv
Copy link
Contributor Author

Continued in #3547

@akashdhruv akashdhruv mentioned this pull request Sep 15, 2023
5 tasks
@akashdhruv akashdhruv closed this Sep 15, 2023
@akashdhruv akashdhruv reopened this Sep 15, 2023
@akashdhruv akashdhruv closed this Sep 15, 2023
@akashdhruv akashdhruv deleted the bittree branch September 18, 2023 07:03
WeiqunZhang added a commit that referenced this pull request Oct 1, 2023
…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>
guj pushed a commit to guj/amrex that referenced this pull request Dec 13, 2023
…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>
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.

5 participants