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

Redoing AFN neutral limiters #8

Open
wants to merge 90 commits into
base: master
Choose a base branch
from
Open

Conversation

mikekryjak
Copy link
Owner

@mikekryjak mikekryjak commented May 30, 2024

Rewriting the AFN neutral limiter implementation to try and isolate the reason for much worsened performance. The BOUT++ version used for this is here: mikekryjak/BOUT-dev#1

Background
Here is our cross-field neutral transport model:
image

This pressure-diffusion approach needs flux limitation to a fraction of free streaming to be physical. Our limiters in Hermes-3 are currently crude and unphysical. Here is how they are being changed:
image

This change introduces a significant slowdown (x10 in small test case to x20 in large production case). This has been isolated to be caused by setting the conduction limiter to be of the new, separate form rather than the previous, Dn derived form.

Note that the new limiters are "smooth" and so should be differentiable. I am using a gamma setting of 4 or 2, which are both very smooth. Gamma is the limiter smoothness (see second slide above). Gamma choice has no impact on the speed.
image

Initial analysis suggested that freezing kappa after the first RHS evaluation, or in the linear solver, can offer significant speedups. This turned out to be an artifact of improperly defined test cases. Here is a comparison of the base (old limiters), new limiters with boolean function, new limiters with asymptotic (smooth) function, and freezing kappa and/or Dn in the linear and total solve:
image

The current effort to debug this is focused around post-processing CVODE residuals. An initial effort with PVODE was tricky because PVODE is ancient and doesn't support preconditioning in the same way, and the slow case ended up faster. The solver also slowed down over time and sped up after a restart, suggesting the solver needs tuning:
image

mikekryjak and others added 30 commits December 13, 2023 13:29
- This allows other components to pick and choose which ones are used rather than using the total frequency all the time. I am leaving the total frequency in there to be refactored later
Apply limiter/upwinding to both X and Y advection
Seems to work better for neutral perpendicular diffusion than MC.
Take field (Nn, Pn, NVn) and Dnn separately; only upwind the field,
and use cell face average values for Dnn.

Also add neutral_conduction switch.
target settings used in SOL and PFR regions, rather than SOL and PFR
settings.
Seems to help with smoothness of profiles
Same as anomalous_diffusion, obtain flows in X and Y for output
when diagnose = true.

Note: Only saves perpendicular flux, not parallel flux.
- For debugging purposes
- added setting for pressure floor. Added temporary debug flags for using floored Nn and Pn in DnnNn and DnnPn calculation (otherwise they could unbalance and lead to density loss - need more testing). There is a secondary floor which kicks in at zero gradient, this is now linked to the primary floor (but 2 orders of magnitude below).
- The collision sums contain exactly the same collisions as they did in the past, allowing legacy behaviour. Note this means that collision_frequency != sum(collision_frequencies).
- Makes it easier to split them up later
- Collisionality mode is now a string input to allow different combinations. It will throw an exception if the string is not "legacy" or "braginskii". Still not sure if "legacy" should be called something else or not.
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