Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add contributor and style guide #99

Open
rileyjmurray opened this issue Jun 26, 2024 · 1 comment
Open

Add contributor and style guide #99

rileyjmurray opened this issue Jun 26, 2024 · 1 comment

Comments

@rileyjmurray
Copy link
Contributor

rileyjmurray commented Jun 26, 2024

We need a contributor/style guide.

What C++ is and isn't on the table

Use of the c++ standard library in RandBLAS is subject to two constraints.

  1. No use of std::random anywhere, except possibly in our test suite. All random number generation (outside select cases in testing infrastructure) needs to happen with Random123 and be mediated by the RandBLAS-defined ``RNGState'' type.
  2. No c++ data structures appear in the public API. So, no std::vector in the signatures of public functions. The public API is defined as what we put in the web documentation.

Dev infrastructure

Don't rely on Homebrew. Use spack.

Homebrew bit me in the butt today, so I bit the bullet and downloaded Spack. It's a lifesaver! Here's how you'd "install" in bash:

export HOMEDIR="~"
# ^ Ideally you'd use a full-qualified path, like /users/riley/.
cd $HOMEDIR
git clone https://github.com/spack/spack.git
# ^ you'll now have a spack folder in your home directory.
printf "\nif [ -f ~/.bashrc ]; then \n\tsource ~/.bashrc\n\n" >> .bash_profile
# ^ This is optional, and can be skipped if the results of the printf are already in your bash profile.
#     The "~/" shortcut to your home directory is okay in this context.
printf "\n. ${HOMEDIR}/spack/share/spack/setup-env.sh\n" >> .bashrc

After that setup, spack should be a valid command in any (standard) terminal you launch.

Once you have spack installed you need to get libraries. Spack recently started to release binaries, which makes installing packages waaaaay faster. Here's the old-school and potentially slow way of doing it:

spack install llvm
# ^ That will run for at least half an hour on a macbook with an M2 Max CPU.
spack install cmake
# ^ That should be pretty quick.
spack load llvm
spack load cmake

Style: brace and parenthesis placement

For-loops, while-loop, and if-statements.

for (int i = 0; i < n; ++i) { 
    // do the thing
}

Function definitions

int foo(int a, int b) {
    // implementation
}

Dealing with painfully long signatures

template <SketchingOperator SKOP, typename T = SKOP::scalar_t>
inline void sketch_symmetric(
    // B = alpha*S*A + beta*B
    blas::Layout layout,
    int64_t d, // number of rows in B
    int64_t n, // number of columns in B
    T alpha,
    SKOP &S,
    int64_t ro_s,
    int64_t co_s,
    const T* A,
    int64_t lda,
    T beta,
    T* B,
    int64_t ldb,
    T sym_check_tol = 0
) {
    // code
}

Style: function docstrings

Docstrings for functions that appear on our web docs take the following form (the long line of equality signs can be
replaced by a line of hyphens when documenting a member of a struct or ).

// =============================================================================
/// This will be parsed by sphinx-breathe. 
///
/// Functions with only a few arguments should be documented in plain exposition.
///
/// I (Riley) don't like how doxyen macros like @tparam, @param, or @returns get 
/// rendered on our web docs. So right now we don't use them. 
///
/// If a function has many arguments then we sometimes write doxygen-like docs in embedded rST.
/// Here's what that embedded rST might look like (see sketch_vector or sketch_general for 
/// more meaningful examples).
/// @verbatim embed:rst:leading-slashes
///      a - [in]
///       * A positive integer.
///
///      b - [in, out]
///       * On entry: An integer.
///       * On exit: <some function of (a,b)>.
/// @endverbatim
/// IIRC, there can't be a blank line after an @endverbatim.
///
/// It's important to use the formatting above. I'd *like* to switch to proper doxygen someday, and 
/// if we're consistent about our doxygen-like rST comments then we can automate the translation
/// into doxygen when the time comes.
///
void mathfunc(int a, int &b) {
    // code 
}

In-line math expressions are typeset with \math{expr} (this is a custom doxygen alias defined in our doxyfile). Punctuation for the surrounding sentence should appear inside the macro when relevant.

Custom LaTeX macros can be defined at the start of a docstring, like this:

// =============================================================================
/// @verbatim embed:rst:leading-slashes
///
///   .. |vals|  mathmacro:: \mathtt{vals}
///   .. |rows|  mathmacro:: \mathtt{rows}
///   .. |cols|  mathmacro:: \mathtt{cols}
///   .. |Dfullnnz| mathmacro:: {\mathcal{D}\mathtt{.full\_nnz}}
///
/// @endverbatim
/// On entry, \math{\rows, \cols, \text{ and } \vals} are null references.
/// On exit, they point to new arrays of size \math{\Dfullnnz.}

I prefer to define math macros in rST files within rtd/source/, since that way one definition can be used for many functions.

Style: misc documentation

I like to use \\ MARK: <text here> as a tool for making source code more navigable. This is mostly in tests, but I'd like to add it elsewhere.

@rileyjmurray rileyjmurray changed the title Use MARK in source comments Add contributor and style guide Sep 10, 2024
@rileyjmurray rileyjmurray pinned this issue Oct 3, 2024
@rileyjmurray
Copy link
Contributor Author

Note to self: see https://github.com/tlapack/tlapack/blob/master/CONTRIBUTING.md for a nice model.

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

No branches or pull requests

1 participant