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

implement full well-balancing in PPM #2945

Merged
merged 16 commits into from
Oct 24, 2024

Conversation

zingale
Copy link
Member

@zingale zingale commented Aug 20, 2024

PR summary

PR motivation

PR checklist

  • test suite needs to be run on this PR
  • this PR will change answers in the test suite to more than roundoff level
  • all newly-added functions have docstrings as per the coding conventions
  • the CHANGES file has been updated, if appropriate
  • if appropriate, this change is described in the docs

the best results now come from use_pslope and reflecting BCs
this is because of work we've done since the README was last
updated.
@zingale
Copy link
Member Author

zingale commented Aug 21, 2024

this seems to work

@zingale
Copy link
Member Author

zingale commented Aug 21, 2024

This seems to work. Here's a version of bubble_convergence using the new ppm_well_balanced:

bubble_ppm_full_well_balancing

and here it is with normal PPM and reflecting BCs:

bubble_ppm_reflect

@zingale zingale changed the title [WIP] implement full well-balancing in PPM implement full well-balancing in PPM Aug 21, 2024
Source/hydro/ppm.H Outdated Show resolved Hide resolved
@@ -459,6 +477,10 @@ Castro::trace_ppm(const Box& bx,
}


// for well-balanced, the velocity sources should not be added

amrex::Real fac = (castro::ppm_well_balanced && in_hse) ? 0.0_rt : 1.0_rt;
Copy link
Collaborator

@zhichen3 zhichen3 Oct 23, 2024

Choose a reason for hiding this comment

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

is it better to just skip the source trace tracing for src_un above if we use ppm_well_balaneced and in_hse?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess so, but somehow I thought this was more clear when reading the code.

Real trho[nslp];
Real src[nslp];

load_stencil(q_arr, idir, i, j, k, QRHO, trho);
load_stencil(srcQ, idir, i, j, k, QUN, src);

ppm_reconstruct_pslope(trho, s, src, flat, dL, sm, sp);
in_hse = ppm_reconstruct_pslope(trho, s, src, flat, dx[idir], sm, sp);
Copy link
Collaborator

Choose a reason for hiding this comment

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

probably better to pass in dL rather than dx[idir] although it only makes a difference in spherical-theta direction but gravity is in r so there will be no difference, but I still feel its more consistent this way. Same goes with using dL to get p_m_hse and p_p_hse later.

Copy link
Member Author

Choose a reason for hiding this comment

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

done

@zingale zingale merged commit db92cba into AMReX-Astro:development Oct 24, 2024
25 checks passed
@zingale zingale deleted the ppm_full_well_balancing branch October 24, 2024 21:45
zhichen3 added a commit to zhichen3/Castro that referenced this pull request Oct 30, 2024
zhichen3 added a commit to zhichen3/Castro that referenced this pull request Oct 30, 2024
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.

2 participants