ammarhakim / gkylzero

Lowest, compiled layer of Gkeyll infrastructure.
MIT License
21 stars 4 forks source link

Enforce consistency in naming and structure conventions #270

Open JonathanGorard opened 8 months ago

JonathanGorard commented 8 months ago

Alright, so I’m currently going through g0 and trying to enforce some consistency in how things are named and structured (in the hope of eventually being able to produce a reasonably canonical guide/glossary to the code). I appreciate that this seems utterly tedious, and is not likely to make me many friends, but it also seems potentially somewhat useful. However, everyone here knows the code much better than me, so your opinions regarding these things are also crucial. I’m going to keep a thread here of (apparent?) inconsistencies that I find, and what canonical form I’m choosing, but if anyone has other views (or feels that I’ve fundantally misunderstood something, which is very likely), then please do let me know! From my work so far on the moment and Vlasov apps:

  1. elcMass/elcCharge/ionMass/ionCharge vs. massElc/chargeElc/massIon/chargeIon: I’m choosing right now to canonicalize everything to elcMass/elcCharge/ionMass/ionCharge;
  2. beta vs. plasmaBeta: I'm choosing right now to canonicalize everything to beta;
  3. sometimes using the "2" suffix to denote a second quantity (e.g. T_i1 and T_i2 to denote magnetospheric and magnetosheath ion temperatures, respectively) and sometimes to denote a squaring operation (e.g. sech2): I'm choosing right now to canonicalize all squaring operations to the "Sq" suffix (e.g. sechSq), reserving numerical suffixes for indexing of quantities;
  4. sometimes using underscores for fractions (e.g. T_i1_T_i2) and sometimes using "Over" (e.g. TiOverTe): I'm choosing right now to canonicalize everything to using "Over"

I'll keep this updated as I go!

ammarhakim commented 8 months ago

In general, let us avoid using CamelCase for naming.

JonathanGorard commented 8 months ago

In general, let us avoid using CamelCase for naming.

Would underscores be preferable (e.g. "sech_sq" and "Ti_over_Te")?

manauref commented 8 months ago

Thanks for this effort. Here are some things that we've (inconsistently) tried using in the past:

  1. When we started generating numerous kernels in gkylzero we decided that we would add modifiers/specificity as suffixes following an underscore. So, a trivial example is vlasov_maxwell and vlasov_poisson, where _maxwell and _poisson are the modifiers. I think we've done pretty well following this rule in kernel names, and tried to follow it in the handwritten code (in zero, unit, apps, regression) but didn't do it consistently in the latter. Under this rule, if it's such a thing, I think we would use mass_elc rather than elc_mass (although this depends on interpretation and this may be a bad case to discuss). Similarly, there could be multiple betas, so we could have beta_plasma, beta_poloidal, beta_elc.
  2. Yes, I've also argued against using 2 to mean multiplication, division or raised by 2. In some kernels you'll find that we've used the suffixes D2 and R2 to mean divided and raised by 2. In fact there's a maxima function that finds powers of a variable and automatically generate temporary variables named varR# for a var^#. We could do something like this (D2,R2), but maybe for clarity something like _mul2, _div2, _pow2 is better. Yes, _sq is also in some places.
  3. As Ammar said, we've tried to not use CamelCase in gkylzero, but we haven't been consistent about it.
JonathanGorard commented 8 months ago

Thanks Mana! That's super useful to know - I like the general principle of the format being something like "quantity_specifier". I will try to enforce that wherever it seems sensible to do so. I feel as though "_mul2", "_div2", etc. are somewhat clearer suffixes than "D2" and "R2", and squaring seems like a sufficiently common operation that "_sq" is not unreasonable in such cases. I think that, given these rules, we can mostly (if not entirely) remove the instances of CamelCase that I've seen in g0 so far.

[Also, for what it's worth, I've also noticed discrepancies between things like "momzi" and "izmom" (for ion z-momentum). I'm currently canonicalizing to the first form, but, as usual, let me know if you have thoughts.]