-
Notifications
You must be signed in to change notification settings - Fork 113
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 parabolic terms in 2D & 3D on TreeMeshes #1629
AMR for parabolic terms in 2D & 3D on TreeMeshes #1629
Conversation
… into Parabolic_AMR_1D
… into Parabolic_AMR_1D
Co-authored-by: Michael Schlottke-Lakemper <[email protected]>
… into Parabolic_AMR_1D
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.
Great addition! Looks very good overall, I have left a few - mostly minor - comments and suggestions.
Co-authored-by: Michael Schlottke-Lakemper <[email protected]>
Co-authored-by: Michael Schlottke-Lakemper <[email protected]>
…rixi.jl into AMR_Parabolic_2D3D_Tree
@DanielDoehring are there any open issues you would like to resolve, or is this ready for us to review again? |
No, nothing from my side. I plan to open the issues we discussed once this is merged to be able to reference the actual code. |
…rixi.jl into AMR_Parabolic_2D3D_Tree
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.
LGTM. Thanks a lot for this super useful new feature!
If @ranocha agrees as well, this can be merged imho
Ok, please request a review when ready then |
Ah, I'm sorry. I missed that @ranocha was not involved in the previous review process of this PR, I mixed this up with another PR. Hendrik, I do not think that it is super necessary for you to review this PR then, since most of the structural decisions had already been introduced by the 1D AMR PR. Just let us know if you plan to have a look, otherwise we can go ahead and merge this once tests pass. |
Related to the work packages mentioned in #1147
This is supposed to be merged after #1602, since it is essentially a superset of the 1D case.
The new elixir
elixir_navierstokes_shearlayer_amr.jl
serves as a neat motivation for velocity-based indicators (see the changes in the compressible euler equations) and tree-AMR.Allocations occur only in the AMR section:
Summary CB