Skip to content

GK passive advection & twistshift fixes - #1084

Merged
JunoRavin merged 22 commits into
mainfrom
gk_passive
Aug 5, 2026
Merged

GK passive advection & twistshift fixes#1084
JunoRavin merged 22 commits into
mainfrom
gk_passive

Conversation

@manauref

@manauref manauref commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

This goes with:

Changes

We add a new passive advection collisionless equation type, GKYL_GK_COLLISIONLESS_PASSIVE. This can be invoked from the input file along with user-specified advection speeds. See rt_gk_passive_3x2v_p1.c, for example, where we use

    .collisionless = {
      .type = GKYL_GK_COLLISIONLESS_PASSIVE,
      .passive_speeds = passive_velocity_elc,
      .passive_speeds_ctx = &ctx,
    },

and passive_velocity_elc is a function that returns 3 values for the x,y,z components of the advection velocity.

This object has been quite useful for debugging some twist-shift BC issues. For example, we can now reproduce the test in figure 14 of M.Francisquez, et al. CPC 298 (2024) 109109:

Screenshot 2026-07-10 at 9 03 53 PM

and now there's a unit test with this set up.

These kinds of tests allowed us to make 3 improvements in twistshift:

  1. There were a few subcell integrals that were not checking if the subcell region had zero volume, which was leading to NaNs for the grid and shift in the published passive advection case. Zero volume subcell regions can happen due to floating-point precision, but should not contribute anything.
  2. There were some cases in which points defining the subcell region were attributed to different periodic copies of the domain, leading to checkerboarding. This was fixed by ensuring subcell regions are always attributed to the same periodic copy of the domain.
  3. The twistshift updater did not complain when the shift wrapped around Ly in a single x-cell. But it was not designed to support this, and it was leading to silent checkerboarding. Now it complains and prompts the user to change q, Ly, or Nx.

However, because of 3 now the d3d and tcv IWL reg tests don't run (because they are too coarse in x). Maybe we should increase the resolution of those.

Other small but consequential changes:

  • We fixed the CBC regression tests, they were setting the y domain incorrectly, leading to erroneous shifts at the twist-shift boundary.
  • We fixed a small bug in gkyl_rect_grid_find_cell. Parts of it assumed the indexing of cells went from 1 to N, but in one place it placed the lower cell at 0.

Additional changes:

  • We renamed gkyl_bc_twistshift_new to gkyl_bc_twistshift_inew, and implemented a new gkyl_bc_twistshift_new which takes the arguments in the function signature, not in an input struct.
  • We also changed some inputs to ``gkyl_bc_twistshift` so that they are passed by address.

These changes make it easier to call bc_twistshift from python/postgkyl.

Results

In addition to reproducing the passive advection test in the twistshift paper, we were also able to check advection across the twistshift boundary in the CBC geometry (see rt_gk_cbc_passive_3x2v_p1.c). Here's a movie made with the gk-rz command in PR 214 of postgkyl:

rt_gk_cbc_passive_3x2v_p1-elc_M0.1.mp4

Moreover, with the changes made to the bc_twistshift API, we created a Marimo notebook that can plot the effect of the shift for a specific input file (see postgkyl PR 224). It allows us to visualize the shifted lower-y cell, e.g.

Screenshot 2026-07-10 at 4 57 41 PM

and a specified quantity, twistshifted by the bc_twistshift operator:

Screenshot 2026-07-10 at 4 57 15 PM

Lastly, the pre-existent bc_twistshift unit tests continue to pass.

manauref added 15 commits July 3, 2026 15:30
… given in the input file. Add sample regression test (gkylcas 16cfe76398a327fa8617ed162509585f0c6c6f47).
…up. We should make it so that omega_H is only computed if the field evolves. Passive eqn object is clearly not ready as the result is not as expected.
…x2v passive advection reg test. Tried using TS BCs in passive advection but I think there's a memory bug, need to switch to cluster with valgrind.
…oken its use in bc_twistshift. Also, I think there was a bug in find_cell all along, though I don't know if it was ever triggered. Fix this bug, and adjust a tolerance in twistshift to make it work with the recently added epsilon in rect_grid_find_cell. Unit tests pass. There's still an issue with passive advection using TS BCs because the solution quickly becomes NaNs.
… show that the shift at the lower boundary is OK, but the shift at the upper boundary (which -1 times the lower shift) produces NaNs.
… integrals of bc_twistshift, it was causing NaNs in the fig14 scenario. Fix boundary kernel selection in gk_collisionless_passive_flux. Now the passive advection test with TS BCs looks as expected.
…uadratic q has issues near the upper radial boundary. We'll examine this in a unit test setting, and try it with TCV geo.
… delete or simplify this in the future, but it already reproduces the checkerboard pattern seen in the simulation.
…in the shift across a single cell is equal or greater to the length of the domain along y. I tried to add support for this sort of scenario but didn't succeed. I'll try again in the future. For now we must increase resolution along y, reduce shear or increase Ly so this doesn't happen. Add accepted results to CBC unit test.
…n TS BC updater. The previous approach looked in periodic copies of the domain independently for each point, but this could give different periodic copies for 2-4 points that are meant to define a subcell integral, leading to error. So now we look for the correct periodic copy only once (for a given donor-target pair) and pass that to be used for looking for intersections. This fixes some checkerboarding seen in the CBC geo with Nx cells. All other unit tests continue to pass (I had to generate new cbc accepted results, but I confirmed visually that for Nx=32 the difference was machine precision.
…eger. This explains a mysterious shift we were seeing when structures crossed the IMP.
…ddress, this makes it easier to call it from python. Rename num_cell_ variables in tcv reg tests so that they use Nx like all other reg tests (makes it easier to automate changes to them).
@Antoinehoff

Copy link
Copy Markdown
Collaborator

Great addition and fixes. Can you share your results of production-like simulation with that branch? Once this is done I think we can merge.

@manauref
manauref requested a review from tnbernard July 24, 2026 19:20
@manauref

Copy link
Copy Markdown
Collaborator Author

Great addition and fixes. Can you share your results of production-like simulation with that branch? Once this is done I think we can merge.

I checked a production TCV and D3D setup, and for those resolutions and shifts/q-profiles, nothing seemed to change. The TS fixes here appears to have afflicted other scenarios, but not those two.

I'll valgrind check when Perlmutter comes back online.

@Antoinehoff Antoinehoff mentioned this pull request Jul 28, 2026
@manauref

manauref commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author
  • the TS BC unit test is valgrind and compute-sanitizer clean.
  • the d3d and tcv IWL reg tests are compute-sanitizer clean.
  • the d3d and tcv IWL reg test is valgrind clean.

@Antoinehoff Antoinehoff left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

thanks !

@JunoRavin
JunoRavin merged commit 3bd8bf7 into main Aug 5, 2026
1 check passed
@manauref
manauref deleted the gk_passive branch August 5, 2026 21:02
@Antoinehoff
Antoinehoff restored the gk_passive branch September 1, 2026 19:12
Antoinehoff added a commit that referenced this pull request Sep 2, 2026
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.

3 participants