Naming

[helenarichie]

This is where we'll discuss our preferences for a unified naming scheme in Cholla.

Per our rules on the other style wars thread:
Voting and discussing:
👍 = "I like this"
😕 = "I don't like this"
👀 = "I have seen this and don't care"
👎 = "I veto this"

To keep voting organized, make each choice a Comment on the Discussion. The top level Comments should all be voting options or a neutral introduction to the topic. Debates should all happen in Replies: general replies under the neutral introduction, option-specific pros/cons under their vote Comment.

Thus the hierarchy is as follows: Discussions (for each issue), Comments (for each option), Replies (for each pro/con)

[helenarichie]

Summary of Naming Schemes from Different Style Guides

Here's a summary of the different naming schemes for the three style guides that we're most interested in adopting. If there are any others that we're interested in please let me know and I'll add them!

In brief, Google is the only guide of these three that has a dedicated naming section, the other guides just adopt Google and have (mostly minor) noted exceptions scattered throughout them.

Google C++

• File names: filenames should be all lowercase and can include underscores (_) or dashes (-)
• Variables (including function parameters): snake_case
  • Constant variables: kPascalCase (title case with a leading lowercase “k”)
• Classes, structs, typedefs, aliases, enums: PascalCase
  • Class data members (static and non-static): snake_case_ (snake case with a trailing underscore)
  • Struct data members (static and non-static): snake_case
  • Accessors and mutators: follow variable naming
• Function names: PascalCase()
• Namespaces: snake_case
  • Top-level namespace names are based on the project name.
• Macros: UPPER_SNAKE_CASE

Chromium C++
Adopts Google with noted exceptions:

• Inline functions: snake_case()

Mozilla C++
Follows Google C++ with noted exceptions:

• Variables: snake_case
  • Global variables: gPascalCase
  • Argument variables: aPascalCase
  • Class/struct static members: sPascalCase
  • Class/struct members: mPascalCase
  • Enum variants: ePascalCase (e.g. enum Foo { eBar, eBaz })
    • Note: this only applies to variants, enum classes should use PascalCase
• Classes, structs, typedefs, aliases, enums: PascalCase
  • Method names: PascalCase()
    • Getters that never fail, and never return null are named Foo(), while other getters use GetFoo()

[alwinm]

Votes for Google C++

[helenarichie]

Would it be accurate to describe Google C++ as snake_case for data (anything that can be x = ), CamelCase for definitions (like defining classes/function)? It is a bit hard for me to remember each rule individually.

Not quite, because class methods for example follow a different naming than the other "definitions" (they follow variable naming)

[bcaddy]

@alwinm, IMO part of the goal of automating the checking is so you don't have to remember. You can write some stuff then run clang-tidy and it will tell you what is wrong. Paired with a simple identifier refactoring tool (included in most IDE's and VSC) then renaming later is easy.

[bcaddy]

I'm generally in favor of Google as well with a couple of points I'd like to discuss, I'll link the threads for them shortly.

• I generally prefer camelCase for variables and PascalCase or snake_case for functions. (link)
• I like k for constant variables, this is good
• I think we should have a way of identifying global variables. (link)
• I'd like to be able to distinguish public vs. private member variables at a glance. IMO we should do this with underscores (link)

Google seems to be the standard for most places and I agree that they have the clearest documentation on it.

[trey-ornl]

One thing that none of these styles considers that might be useful is for names to indicate host-resident versus device-resident variables. That may be something you can just append to whatever standard you choose.

[bcaddy]

Excellent point. We currently pre-append with dev_ for that usually and I think we should continue something like that.

[alwinm]

Votes for Chromium C++

[bcaddy]

IMO it's Google with one change for an edge case that isn't really an issue for us and so we shouldn't consider it. Inline vs. linked functions matters for performance and naming them distinctly is important if the codebase is very large. Our codebase isn't that large and almost all our performance critical functions are __device__ functions which are all inlined anyway.

[helenarichie]

I agree that the naming distinction for inline functions isn't important, I just wanted to include it to be thorough.

I think that the only distinction between a vote for Chromium and a vote for Google for the purposes of naming is having the style guide as a reference. Like I mentioned, Chromium doesn't have a dedicated naming section in the context of their other style choices, so it's just less convenient than having a consistent dedicated naming section within the style guide.

[alwinm]

Votes for Mozilla C++

[helenarichie]

Assessing the Mozilla style guide from a naming perspective, I feel that this wouldn't be a great choice. They don't have a dedicated naming section, and instead follow Google with noted exceptions scattered throughout their style guide when they're talking about a particular topic. This makes their naming suggestions difficult to find, so we wouldn't have a quick reference. On top of this, I don't think that their suggested alterations to the Google style guide are especially helpful.

[bcaddy]

I agree generally. It's less clear, refers to google as a primary, and generally doesn't bring much to the table

[bcaddy]

camelCase for variables and PascalCase or snake_case for functions.

[bcaddy]

IMO camelCase is easier to type and since we spend more time typing variables than functions it should be preferred. Cholla already uses a lot of camelCase for variables, PascalCase and Loud_Snake_Case¹ for functions.

Footnotes

1. Does this have a proper name? ↩

[helenarichie]

To me camelCase can be hard to read. I also don't really agree that it's that much easier to type than e.g. snake_case.

I think that Loud_Snake_Case is a good choice for functions since it's readable and the way that a lot of Cholla functions are already named

[bcaddy]

Variables: fair enough, I don't mInd a ton either way and snake_case definitely makes prefixes and suffices easier to read.

Functions: IMO functions and variables should be more different so if we're using snake_case for variables then functions should be PascalCase

[alwinm]

In my view, snake_case for variables and Loud_Snake_Case for functions/classes/etc is a light perturbation on the google C++ guide and something I can get behind. Consistent underscores everywhere makes sense to me, my logic being: if underscores make it easier to read, why not use them everywhere? But I can also see the case for using PascalCase instead. No strong feeling--I personally find the capitalization provides sufficient distinguishing power in either case.

[evaneschneider]

Something I don't understand at this point in the discussion. How rigid will adherence to an adopted style guide be? Considering variable names and capitalization, for example. Let's say you want to define the log of the temperature and the log of the density in a single function. I think naming those variables log_d and log_T is sensible and easy to understand. But that doesn't seem like it would be an option with any of the listed style guides. I don't want to lose the easy-to-interpret physics of the code, in order to adhere to a specific style.

[helenarichie]

Something we had discussed in person and were planning to introduce to this discussion is coming up with standardized names for physical variables, which would definitely be exceptions to any style guide we went with

[bcaddy]

I think this might be a case of we pick a rule and then add exceptions as needed.

I agree that log_T is way better that log_t and we can add exceptions to clang-tidy so it will allow that

[bcaddy]

@helenarichie, I created a thread for naming of physical variables here

[alwinm]

In general I think there will be some things that we enforce and some things where maybe there is a preference but not a hard enforcement, and some things where we don't make any decision. I lean towards a gradual adoption where we start enforcing some of the more popular value-providing options first, and adopt as we go.

[bcaddy]

I agree. And I think we should focus on things that are able to be automated since that is easier and more scalable. I.e. requiring snake_case variables is easy to automatically enforce and the codebase can be transitioned semi-automatically with refactoring tools but moving all old references go physical variables to the new standard is more manual and can't be automated so we should only loosely enforce it.

If we can't automate a specific requirement then IMO at most we should have guidelines rather than rules since requiring a lot of human intervention isn't sustainable.

[bcaddy]

Global Variables should have a suffix or prefix that indicates they're a global

It looks like the consensus is a g_ prefix

[evaneschneider]

Maybe g_? I'm just immediately imagining the unpleasantness of ggama everywhere...

[bcaddy]

I agree. It looks like we're converging on snake_case for variables anyway so g_ makes sense and matches our use of dev_ for device variables.

[alwinm]

I see how the g_ will provide value. IMO it is kind of ugly but provides impetus to get rid of globals. no strong feeling

[evaneschneider]

My sense is that most of our globals are array pointers. Would this be appended to those, or only single variables? I don't love e.g. g_dev_conserved everywhere...

[bcaddy]

I'd prefer it be added to all globals. g_dev_conserved is ugly but both of the prefixes contain useful information and I think we should minimize the number of exceptions we have to rules.

Also, most of our global arrays can, and should, become static local variables. With static DeviceVector objects and the new reductions it should be fairly easy to remove most globals.

[bcaddy]

Public and Private member variables

Proposal:

• Public member variables use the standard variable naming convention
• Private member variables use the standard variable naming convention with a trailing underscore

[bcaddy]

The Google style guide is unclear on this point. They say all member variables should have an underscore suffix like my_var_ but all the actual code they have only uses that for private member variables and uses snake_case for public ones. I've found being able to quickly distinguish private vs. public member variables quite handy. So I suggest that:

• Public member variables use the standard variable naming convention
• Private member variables use the standard variable naming convention with a trailing underscore

[bcaddy]

@evaneschneider, @helenarichie, @alwinm, @ojwg.

I think we should reverse our decision to use an underscore prefix for private members and have an underscore suffix instead. Namely because C++ reserves all names that start with two underscores, an underscore and a capital letter, and an underscore in global namespace. The second one potentially runs afoul of several of our other naming conventions, especially the function naming conventions for private member methods.

cppreference identifiers

[helenarichie]

I see. If that's the case, a suffix for private variables sounds like a good solution to me.

[bcaddy]

Prefix for Device Variables and Functions

Proposal:

• dev_ prefix for device variables and nothing for device functions.
• Global device variables should be g_dev_. I know it's ugly but clang tidy can't enforce it if it's in the other order

[bcaddy]

d_ and h_ are also used for device and host variables. Thoughts on that vs. dev_?

[evaneschneider]

I think dev and host are clearer.

[bcaddy]

Agreed. And we should only use host_ when two arrays/variables mirror each other?

[trey-ornl]

To clarify, I would say that dev_ means, "I am a pointer to memory that has been allocated on the device, or I am an object with pointer(s) to such memory".

[bcaddy]

That's what I meant, sorry I wasn't clear.

[bcaddy]

Kernel Naming Convention

Special naming for kernels is not currently enforceable by clang-tidy so I think they should follow standard function conventions

[alwinm]

Agreed that it should mostly be the same as functions, but I like it when kernels have "kernel" in the name. Not sure if that should be enforced.

[bcaddy]

Maybe end it _kernel? This can't be enforced with clang-tidy at this time and I am loathe to add things we can't automatically enforce

[alwinm]

In that case, I like not having it be enforced.

[helenarichie]

I think it would be useful to have a distinction between functions and kernels, even if it can't be automatically enforced.

[bcaddy]

I agree that it would be nice, but IMO in practice if we can't automatically enforce it then it probably won't be done consistently which I would argue is worse than not having an identifier at all. Since it will throw a compile time error if used wrong I'm ok with not having a different naming scheme for kernels

[bcaddy]

Names For Specific Kinds of Variables

I.e. density, momentum, etc. Note that clang-tidy cannot enforce this, it has to be enforced via code reviews.

[bcaddy]

My proposal:

Following variable naming convention as a base. Spell out the full name of the thing followed by appropriate modifier for the specific thing

• Density: density
• Momentum: momentum
• Velocity: velocity
• Energy: energy, energy_gas, energy_kinetic, energy_magnetic, energy_total
• Pressure: pressure, pressure_gas, pressure_magnetic
• Scalar: scalar
• Temperature: temperature. I'm a bit torn on this one. It's long but T and temp are already used commonly as template parameters and "temporary" respectively
• Magnetic Field: magnetic
• Electric Field: electric
• Fluxes: flux when it contains many different flux fields and field_flux for specific fields. Example: flux arrays in the integrators would be flux_x and the density flux in a riemann solver would be density_flux
• Wave speeds: speed_wave_name
• Vector direction: append the direction like velocity_x
• Number Density: num_density?
• CUDA/index IDs: thread_id, id_x, id_y, id_z (maybe xid since we already use that a lot, it technically breaks the other rules but might be a good exception)

I prefer this over @helenarichie's suggestion because single/few letter names are unclear. Example: m shows up everywhere (mass, momentum, magnetic, magnitude, meter, milli), n means lots of things, T is common template parameter name.

I do like log_ for log values but I think it should be a suffix since we mostly care about what we're taking the log of not that it is a log, this is consistent with Google and LSST style guides. So density_log instead of log_density. I know the latter is more Englishy but I think the former is clearer for code.. I think that can be easily extended for other operations, i.e. density_squared.

[bcaddy]

dt and dti are fine since they don't conflict with anything else but it would be nice for them to be spelled out. As long as they're documented I don't really care though. Same for dx, dy, dz. I'm generally more ok with Header member variables having short names since they're so simple and ubiquitous

[alwinm]

This is one where the code is so split that I think it is fine to allow both a long-name and a short-name. Probably one of the things we would enforce later rather than sooner. Is there a pattern in the current codebase that we could start soft-enforcing? It feels to me like short-names for local variables in functions, long-names for names in functions (imagine calc_energy vs calc_E) and long-lived variables like C.d_density, output names.

[bcaddy]

I agree that this is more a long term goal than a short one. At least having a specific conventions for new code would be nice though and old code can be updated as it is edited/refactored.

I'd really like to have one convention for variables and functions, I think that will be less confusing. Longer format names are also clearer when you have to have multiple variants of them. This has been a real pain point with me and MHD since I have multiple different energies and pressures, one each for gas, magnetic field, and both. Plus, and I can't stress this enough, a lot of <3 letter names use common letters that are used in many different things

[bcaddy]

December 16th, 2022 meeting notes on naming

• Density: density
• Momentum: momentum
• Velocity: velocity
• Energy: energy, gas_energy, kinetic_energy, magnetic_energy, total_energy
• Pressure: pressure, gas_pressure, magnetic_pressure
• Scalar: scalar
• Temperature: temperature. I'm a bit torn on this one. It's long but T and temp are already used commonly as template parameters and "temporary" respectively
• Magnetic Field: magnetic
• Electric Field: electric
• Fluxes: flux when it contains many different flux fields and field_flux for specific fields. Example: flux arrays in the integrators would be flux_x and the density flux in a riemann solver would be density_flux
• Wave speeds: speed_wave_name
• Vector direction: append the direction like velocity_x
• Number Density: number_density?
• CUDA/index IDs: thread_id, id_x, id_y, id_z (maybe xid since we already use that a lot, it technically breaks the other rules but might be a good exception)
• device arrays prefix with dev_
• host arrays that mirror a device array prefix with host_
• log_, sine_, etc for the log, sine, etc of a variable
• Equations should have citations

I'm still in favor of energy_gas over gas_energy.

[bcaddy]

Filenames

Google allows snake_case or kebab-case. Most of our stuff is snake_case so I think we should just only allow that.

[alwinm]

Meeting Dec 16 notes

Loud_Snake_Case for functions
snake_case for variables
PascalCase for class/struct definitions
prefix _ for private variables

[helenarichie]

See the Cholla Wiki for a summary of this discussion: https://github.com/cholla-hydro/cholla/wiki/Making-a-Pull-Request