Refactor of skipping phase space cell updates

[Maxwell-Rosen]

This is a continuation of #864, where we added skip_cell to the new collisionless updaters. This PR aims to address #878 most of all, where Mana commented that it would be better designed if we determined the mask in one place, then applied it to all the updaters.

This means creating a new updater, called zero/skip_cell.c, which determines the mask.

Array.c is modified to support boolean arrays, reducing memory usage for this project.

Modifications are made to pass this new skip_cell struct to each relevant place.

The input file is modified, so now we specify inside each species.

.skip_cell = {
  .threshold = 1e-16
}

Each species has its own skip-cell updater, which is updated at every RK stage during the Euler step.

Future work may be interested in extending this mask to the moment calculators; however, I'm not sure how to do so within the current infrastructure. For instance, there are many calls to the moment updaters, and I don't know how each one should be changed.

Regression test results showing that the skip_cell works fine.

pgkyl gk_wham_1x2v_p1-elc_cflrate_10.gkyl sel --z2 1.0 pl

pgkyl gk_wham_1x2v_p1-elc_10.gkyl interp sel --z2 1.0 pl --logz --zmin 1e-18

pgkyl gk_wham_1x2v_p1-ion_cflrate_10.gkyl sel --z2 1.0 pl

pgkyl gk_wham_1x2v_p1-ion_10.gkyl interp sel --z2 1.0 pl --logz --zmin 1e-18

[manauref]

Can you add a type to the input file table please? like

.skip_cell = {
  .type = GKYL_GK_SKIP_CELL_F_THRESHOLD,
  .threshold = 1e-16
}

as in the future we may have some other criteria and may implement other types that depend on, say, the Hamiltonian (as we did in force softening), boundaries, etc. Also, important, the 0th entry in said enum should be GKYL_GK_SKIP_CELL_NONE, and the logic in the app should be such that if type==0 then nothing happens with regard to this operator, so that if the input file doesn't specify this table, then the rest of the solver proceeds with little knowledge and no impact of this operator. That's how every other module is implemented by now.

[Maxwell-Rosen]

Great suggestion! That makes the evaluation of the threshold more memory safe.
I should also mention that rt_gk_wham_1x2v_p1 is valgrind clean

[JunoRavin]

Your mask convention is still the opposite of what we're doing for fluids. You're setting the mask value to be 1 if you are inside the mask, but elsewhere in the code we say the mask value is -1 if we are inside the mask.

Please swap the convention to be consistent with moments. -1 means you are inside the mask, 1 means you are outside the mask.

and please use the convention @ammarhakim outlined that the function should be update_cell not mask_eval so that you can use true values everywhere instead of having to check !mask_eval.

[Maxwell-Rosen]

I think this is semantics @JunoRavin. The mask evaluation just answers the question of if the mask is true here. What true means depends on if you specify a less than threshold or a greater than threshold. A boolean true means that this condition is true. We really should do 1=true rather than -1=true. Doing -1=true is so backwards.

I think _eval makes a lot of sense here instead of update_cell, as suggested by @ammarhakim. The update_cell name is very specific to the skipping phase space cell updates application, but this aims to be a general method to evaluate a conditional statement of less than or greater than. I think this conversion of the double to boolean is essentially an evaluation of the array, which is why I called this an eval method, which is a term used in other updaters for a similar purpose.

[JunoRavin]

This is not a boolean mask. It is a mask for determining whether you are inside or outside a region, with the ability to define regions for updating cells or other conditions. At minimum, I need you to switch the convention to match moments. -1 means you are inside the region. 1 means you are outside the region. In this case, 1 being outside the region means we are updating cells and not skipping the updates.

[Maxwell-Rosen]

Okay I realized the issue is not that this mask should return -1 as true, but the skip_cell mask should have 1 for continuing the update. In other words, we should rename this to update_cell and we should use a greater than conditional mask for that update. A cell which gets an update has a true value, in other words f is greater than the threshold is true. The rename to update_cell makes more sense so that we express something in code like "if update_cell then update the cell". The logic was backwards here, but it all agrees with the correct conventions in the code.

wham_1x2v is valgrind clean and reproduces the results in the initial message. There are no issues on the GPU or on CPU, as both reproduce these plots.

Jimmy and I discussed that the skip_cell struct could be used in conjuction with the orbit-average-phase mask to completely skip phase space cell updates in the masked cells, which would save a lot of compute power. Right now, we are computing the full equations, then setting it all to zero, but if we use the skip_cell component of these updaters, then we can get some more performance. That project is left to future work and exceeds the scope of refactoring the phase space cell updates

[Maxwell-Rosen]

My latest changes are improvements to this PR that I realized recently.

The unit test accessed private members of the gkeyll_array and set the data[] vector directly. This is totally wrong, and we should not do this. I improved the unit test to use a use_gpu flag, reducing its length by half.

I realized the source of this mistake was that the gkyl_array_mask structure was public; making it private prevents this type of issue. Moving the struct to be more private required creating a few new functions to access it, but the returned values are usually const. There is an exception for how the devices acquire the pointer to the dev_ptr attribute. This is consistent with how this is done elsewhere in the code, where a pointer to the device struct is passed without an acquire method.

There was some monkey business with the eval function; it needs a separate definition on both the device and the host. Since this utilizes the struct, it cannot be defined in the public header, which does not know about the array_mask struct. I solved this by creating a public host function that calls a device function in the kernel. The cu.cu, and _priv.h files can call this private kernel directly without introducing layering.

The figures from the OP are reproduced. The unit test is valgrind clean. The regression test are compute-sanitizer clean with & without -g on perlmutter.