Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion components/eamxx/.clang-format
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ AlignConsecutiveMacros: true
AlignEscapedNewlines: true
AlignTrailingComments: true
---
# # these are the defaults for the LLVM style, generated by
# # you can obtain the defaults for the LLVM style by running
# # clang-format --style=llvm --dump-config > [outfile]
# # the option definitions are found here:
# # https://clang.llvm.org/docs/ClangFormatStyleOptions.html
55 changes: 55 additions & 0 deletions components/eamxx/docs/developer/style/format.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# EAMxx Code Formatting Standards

To enforce consistent code format throughout EAMxx, we make use of an
autoformatting workflow, carried out via Github Actions in the E3SM repository.

- The tool we employ is
[`clang-format`](https://clang.llvm.org/docs/ClangFormat.html),
and the version we have chosen is v14.[^v14]
<!-- FIXME: Nail this down when figured out -->
- The standard we maintain is largely identical to the
[LLVM Coding Standards](https://llvm.org/docs/CodingStandards.html),
and a list of the handful of customizations of this convention are
enumerated in the configuration file [`$EAMXX_ROOT/.clang-format`](https://github.com/E3SM-Project/E3SM/blob/master/components/eamxx/.clang-format).
- See this [How-to Guide](resources/clang-format_HOWTO.md)
for additional details on how to configure `clang-format` on your chosen
development machine.

## Automated Workflow Summary

- The `eamxx-format` workflow runs automatically and passes or fails based on
adherence to our formatting standard.
- The workflow is triggered by any Pull Request (PR) that modifies EAMxx code.
- It is also triggered by other PR-related triggers, such as pushing
changes, or converting from **draft** to **ready**.
- All code modified by a PR must be formatted prior to **merging**.
- It is not necessary for your code to be formatted upon ***opening*** a
Pull Request, but feel free to `clang-format` as you develop if that is
your preference.
- The one situation for which opening a pre-formatted PR may not be
preferred is if the file has never previously been `clang-format`-ed
and requires a large number of changes.
- I.e., touching the majority of lines in a file for format-only
changes will make it difficult for a reviewer to determine which
lines were changed for substantive reasons.
- In this case, please refrain from formatting the code prior to
opening the PR, and, instead, run `clang-format` once the PR is
approved to be merged and make that the final commit.
- As of now, the `eamxx-format` workflow only considers files that are edited
by the Pull Request.
- In addition to the pass/fail status, the workflow, provides the path to any
files that caused the failure.
- This information is found on the ***Summary*** page for any failed
`eamxx-format` ***Job***.[^huh-where]

[^v14]: It turns out that this is important because there really are
differences in behavior across versions.
[^huh-where]: To get to this summary, select the ***Checks*** tab at the top of
the PR page and select the `eamxx-format` workflow from the left sidebar.
The summary is in the main pane of the page with the title
**clang-format-linter summary**.[^also]
[^also]: Note that this can also be accessed the long way around by following the
***Actions*** link at the top of the E3SM repository page;
select `eamxx-format` from the ***All workflows*** section of the ***Actions***
sidebar; then choose the most recent run that is associated with your PR,
which should be near the top of the list.
66 changes: 66 additions & 0 deletions components/eamxx/docs/developer/style/functions.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
# Functions and Methods

## Naming

- Please name functions (methods) to be ***descriptive*** and ***verb**-ish* so
that the name describes what the function *does*.
- For example, `install_flux_capacitor()` instead of `f_capacitor_method()`
or `ifc()`.
- To note, if your function cannot be named verb-ishly, that is probably a
sign that there is a fundamental issue with your function.
- In general, functions should use `snake_case` and not contain capitalization,
unless capitalizing that quantity is a standard convention (e.g.,
`find_Doc_Brown()`).

## Usage

- In situations where multiple class-member variables are passed as function
arguments, favor passing the object, rather than the individual variables,
for the sake of shorter lines and function signatures.
- As a rule of thumb, developers should prefer passing arguments by reference,
rather than by value, especially in cases for which the argument is ***large***
or structurally ***complex***.
- E.g., prefer `func(Type &x) { ... }` versus `func(Type x) { ... }`.
- This holds true much of the time because passing by value creates a copy
of the argument for use by the function, and this increases memory
pressure and a resultant performance penalty.
- To be more in-depth on the topic, we echo the guidance from the
[***C++ Core Guidelines***](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#S-introduction)
and would encourage the curious to read more deeply on the topic.

> - For "in" parameters, pass cheaply-copied types by value and others by
> reference to `const`.
> - For "in-out" parameters, pass by reference to non-`const`.
> - For "out" parameters, prefer return values to output parameters.

- The authors go on to explain that "cheap to copy" includes variables
holding 2 or 3 words--for example a double, pointer, or reference.
- To illustrate, the below function adheres to the provided guidance.

```c++
SmallStruct func(LargeNComplex &notSmall, double forty_two,
const int *laser, const BigStruct &lotsaData) {

// SmallStruct object is only assigned-to and then returned, so it is
// defined as the return type
SmallStruct ans;
// forty_two, a scalar double, is considered small and so passed by value
ans.val = forty_two;
// lotsaData, a BigStruct object, is only accessed and so is
// passed by const reference
ans.other_val = lotsaData.small_piece;

// we pass laser by value and as const because pointers are cheap to copy,
// and the value is not changed
for (int i = 0; i < forty_two; ++i) {
ans.val_vector.push_back(laser[i] + notSmall.epsilon[i]);
}

// notSmall is large and also modified in the function, so we pass by
// reference
// it is also accessed, above, so it must be an argument and not the
// return value
notSmall.ans_struct = ans;
return ans;
}
```
208 changes: 208 additions & 0 deletions components/eamxx/docs/developer/style/resources/clang-format_HOWTO.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
# How-to Guide: `clang-format`

This guide is for developers who wish to apply `clang-format` on their chosen
development machine, whether that be their personal machine or a multi-user
cluster.
In this guide, we will describe how to configure/run `clang-format` on EAMxx
code and also how to install it if necessary.

## Configure and Run `clang-format`

Running `clang-format` according to the defined EAMxx standard ***can*** be
done using only command line arguments; however, the command is quite long.
The easier route is to reference the configuration file
(`$EAMXX_ROOT/.clang-format`).
In this case the command is

```bash {.copy}
clang-format [-i] --style="file:${EAMXX_ROOT}/.clang-format" <path/to/c++/source/file(s)>
```

where the `-i` (`--in-place`) argument controls whether the formatting edits
are conducted and change the source file(s) ("in place") or the required edits
are printed to `stdout` (flag omitted).
Also, note that the `--style` flag can also fully define the options without
the configuration file as follows, but this is not recommended as the
configuration could change before this guide is updated to reflect as such.

<!-- markdownlint-disable MD046 MD013 -->
??? Danger "Terminal One-(long-)liner"

```bash {.copy}
clang-format [-i] --style="{BasedOnStyle: llvm, ColumnLimit: 100, AlignConsecutiveAssignments: true, AlignConsecutiveBitFields: true, AlignConsecutiveMacros: true, AlignEscapedNewlines: true, AlignTrailingComments: true}" <path/to/c++/source/file(s)>
```
<!-- markdownlint-enable MD046 MD013 -->

## Installing `clang-format`

On a personal machine, which we will consider to be one for which you have
`sudo` privileges, installation can be conducted via package manager or by
building `llvm v14` from scratch.
If you are a non-admin user of a multi-user cluster, HPC platform, etc., then
it is likely to be an ***easy*** process, though potentially not immediate.
Copy link
Contributor

Choose a reason for hiding this comment

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

immediate?

it's pretty ez, tell them to get clang-format from conda!


<!-- markdownlint-disable MD046 -->
??? Note "If you require or already have multiple versions of `clang-format` installed"

Note that, depending on your requirements, this could be placed in your shell
config file (`.bashrc`, `.zshrc`), or if only needed for a shell session, it
can be done directly from the command line.

The other option is to add a versioned symbolic link to your `PATH`.
This is sometimes included in the package's `bin/` directory by default and,
if not, can be added there after of placed somewhere that is already on your
`PATH`.

```c++
$ cd /opt/homebrew/opt/llvm@14/bin
$ ls clang-format*
clang-format
// no versioned binary so we create symlink
$ ln -s ./clang-format ./clang-format-14
$ which clang-format
/opt/homebrew/opt/llvm@14/bin/clang-format-14
```

**OR**

```c++
$ cd /opt/homebrew/opt/llvm@14/bin
$ ls clang-format*
clang-format
// no versioned binary so we create symlink in a directory already on PATH
$ echo $PATH
/usr/bin:/usr/local/bin
$ ln -s ./clang-format /usr/local/bin/clang-format-14
$ which clang-format
/usr/local/bin/clang-format-14
```
<!-- markdownlint-enable MD046 -->

=== "Personal Machine"

<!-- markdownlint-disable MD046 -->
=== "Mac Users (Homebrew Package Manager)"

For Mac users, the [Homebrew](https://brew.sh)
package manager (`brew`) is the quickest and most straightforward way
to get `clang-format` installed.
Since `clang-format` v14 is not available to install directly from
Homebrew, we install the entire LLVM package at version 14, and this
is as simple as

```bash {.copy}
brew install llvm@14
```

It it likely that Homebrew will issue a message about not linking the
`clang`-related tools by default, so next we add the binary to our `PATH`.

```bash {.copy}
$ export PATH="/opt/homebrew/opt/llvm@14/bin/clang-format:$PATH"
# Note: this is the default location for a recent Mac running Apple silicon.
# It may be different on another system.
# You can confirm where yours is installed via 'brew info llvm@14'
$ which clang-format
/opt/homebrew/opt/llvm@14/bin/clang-format
```

Note also, that if your system has multiple version of `clang-format` installed,
it may be preferable to instead set a versioned alias to `clang-format`
(`clang-format-14`) as in

```c++
// create a shell-alias
alias clang-format-14="/opt/homebrew/opt/llvm@14/bin/clang-format"
```

=== "Linux Users (Package Manager)"

Given the many flavors of Linux, it is difficult to generalize, but there
is a high probability the proper version of `clang-format` or `llvm` is
provided by the built-in package manager.
The commands will differ based on your Linux distribution, but using the
Debian/Ubuntu `apt` syntax, it could be accomplished via something like

```bash {.copy}
$ apt search clang-format
[...]
clang-format-14/...
$ apt install clang-format-14
```

=== "Build from Source"

If you do not succeed in the above, `clang-format` can also be fully built
from the [LLVM Compiler Infrastructure](https://github.com/llvm/llvm-project).
It will begin with something like

```bash {.copy}
git clone git@github.com:llvm/llvm-project.git
git checkout llvmorg-14.0.6 # version tag
```

Also, if you only need `clang-format` and not any of the other tooling,
it will build faster/smaller if you use the CMake flag
`-DLLVM_ENABLE_PROJECTS="clang"` to only build `clang` and it's friends,
rather than all of `llvm`.
And finally, the README for [LLVM version 14.0.6](https://github.com/llvm/llvm-project/tree/llvmorg-14.0.6)
is far more comprehensive that the one for the latest version, and it contains
instructions specific to that build.

=== "Multi-user System"

In many cases `llvm`, `clang`, or `clang-format` will be available as a module,
though whether version 14 is an option could be less likely.
In the optimistic case, it could be as simple as (using Lmod syntax)

```bash {.copy}
$ module avail llvm # [clang, clang-format]
[... list ]
$ module load llvm/14.0.6
```

If it is not available, you will probably need to reach out to a system
administrator to get an official version installed.[^but-conda]
<!-- markdownlint-enable MD046 -->

---

<!-- markdownlint-disable MD046 -->
??? Tip "Unnecessary but Convenient Workflow Customization (`direnv`)"

If you'd like to add a layer of automation/complexity to ensure you only use
`clang-format v14` on `EAMxx` and/or want to use a newer version on the rest
of your system, there is a very handy terminal tool called
[direnv](https://direnv.net/)
(`brew install direnv`) that allows you to automatically load and unload
environment variables based on `$PWD` using a `.envrc` file.
As an example, here's my `.envrc` that adds `clang-format v14` to the path
when I'm working on `EAMxx`.

```bash {.copy}
PATH_add /opt/homebrew/opt/llvm@14/bin/clang-format

# also, since I often forget to load a python environment that's required for
# running ctest, this creates or loads a python 3 virtual environment with numpy
layout python3
pip install --upgrade pip
# the upgrade isn't strictly necessary but trades a little extra setup on
# the front end to avoid pip endlessly reminding you to update
pip install numpy
```

This file can be placed in the top-level `EAMxx` directory, and running
`direnv allow` enables the functionality.
Namely, executing `cd EAMxx` loads the defined environment that stays loaded
in any subdirectories, and resets the standard environment when exiting to a
directory above/outside of `EAMxx`.

For the `conda` fans, this tool can also be used to auto-load a
pre-configured `conda` environment since the `.envrc` is essentially a bash
script with a few bells and whistles tacked on.
<!-- markdownlint-enable MD046 -->

<!-- markdownlint-disable MD053 -->
[^but-conda]: There are rumors of using `conda` creatively to do a user-install,
but that is not an option we support or suggest.
Copy link
Contributor

Choose a reason for hiding this comment

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

wait-but-what-?!?! we use conda for everything!!!!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mahf708 - would you like to add a few lines about doing this with conda?

Happy to add it, if you want to send me a few points of instructions

Copy link
Contributor

Choose a reason for hiding this comment

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

No need to add it now. We can revisit later. I am happy with your current version and we can integrate as-is for now :)

<!-- markdownlint-enable MD053 -->
56 changes: 56 additions & 0 deletions components/eamxx/docs/developer/style/style.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# EAMxx Code Style Standards

EAMxx does not currently impose strict styling standards, other than those of
the autoformatter.
However, if we can follow consistent style and usage guidelines, we can make
EAMxx developers' lives easier and turnaround times quicker.
In the age of modern text editors with autocomplete and running our code on
machines with enormous storage capacity, there is no real need to save screen
or disk space with minimal naming strategies or other meaning-obscuring style
choices.
So, please adhere to the spirit of these guidelines, and your fellow developers
will thank you for it!

## General Guidelines

- Please give ***descriptive***, rather than ***terse***, names to your
variables, functions, classes, etc.
- Avoid idiosyncratic naming or styling conventions.
- If the utility of a style decision would not be immediately apparent to
a reasonable developer, please avoid its usage.
- In the hierarchy of coding concerns, correctness and speed take the top
tiers, but below that level, please favor readability and clarity over
space savings, line-breaking, heavy use of arcane language features, etc.
- That said, if an arcane language feature ***is*** the correct tool to
be used, please do so.
- And perhaps add a comment to aid the non-Jedi-Master developers who
read the code next. :slightly_smiling_face:
- With regard to comments, the general wisdom is that the correct amount of
comments is the amount required to make the code clear and easy to understand.
- To quote
[Jeff Atwood](https://blog.codinghorror.com/code-tells-you-how-comments-tell-you-why/)
from his essay about code comments:
> Only at the point where the code *cannot* be made easier to understand
should you begin to add comments.
- However, since that does not offer much in the way of prescriptive
guidance, here are some guidelines gathered from around the internet.
- A general rule of thumb, though not always true, is that comments
should explain ***why*** something is being done in the code,
rather than ***what*** the code is doing.[^butwhattabout]
- Comments should dispel confusion and not cause it.
- Comments should not duplicate the code.[^dupe]
- Provide links or references when using code from elsewhere or
whenever it is otherwise appropriate.
- A bad comment is worse than no comment.
- Add comments to explain non-standard usage or unidiomatic code.

[^butwhattabout]: An obvious exception to this is explaining complex or opaque
parts of the code that cannot be made simpler--for instance, a clever arithmetic
Copy link
Contributor

Choose a reason for hiding this comment

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

or a galaxy-brained ones like below

Q_rsqrt

float Q_rsqrt( float number )
{
	long i;
	float x2, y;
	const float threehalfs = 1.5F;

	x2 = number * 0.5F;
	y  = number;
	i  = * ( long * ) &y;                       // evil floating point bit level hacking
	i  = 0x5f3759df - ( i >> 1 );               // what the fuck?
	y  = * ( float * ) &i;
	y  = y * ( threehalfs - ( x2 * y * y ) );   // 1st iteration
//	y  = y * ( threehalfs - ( x2 * y * y ) );   // 2nd iteration, this can be removed

	return y;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hahahha, the cursed Quake function--pure poetry 🥇

Copy link
Contributor

Choose a reason for hiding this comment

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

Good God, what IS that?!?

trick in a complicated interpolation scheme.
[^dupe]: For example, this type of comment does not add any information and
is unnecessary.

```c++
// perform initialization
this->initialize();
```
Loading