Skip to content

Contribute

Jump to bottom
craffael edited this page Jan 26, 2021 · 18 revisions

You are encouraged to contribute fixes and improvements to the codebase of LehrFEM++. However there are a few things to consider before making a pull request:

  1. Follow our coding standard
  2. Include unit/integration tests if possible

1. Style guide

1.1. Naming conventions from Google C++ Style Guide

  • Filenames should be all lowercase and can include underscores (_) or dashes (-).
  • Type names start with a capital letter and have a capital letter for each new word, with no underscores. For clarity the suffix _t should be appended1.
  • The names of variables (including function parameters) and data members are all lowercase, with underscores between words.
  • Data members of classes (not structs), both static and non-static, are named like ordinary non-member variables, but with a trailing underscore.
  • Regular functions have mixed case; accessors and mutators may be named like variables. Ordinarily, functions should start with a capital letter and have a capital letter for each new word.
  • Namespace names are all lower-case.
  • Macros are all uppercase.
  • Variables declared constexpr or const, and whose value is fixed for the duration of the program, are named with a leading "k" followed by mixed case.

1.2. Conventions for commenting

  • Throughout use /** */ and JavaDoc style for Doxygen documentation, never use // or /* */ for this purpose.
  • Use // comments only for explaining details of variables and algorithms inside functions. You may use LaTeX typesetting elements in // comments.

1.3. Variable names

  • short variable names may be used, but then the variable has to be explained by a comment at the site of declaration.

1.4. clang-format and clang-tidy

LehrFEM++ relies on clang-format and clang-tidy to make sure the codebase conforms to the styleguide described above. Whenver you make a pull request or push a commit to the repository, these two tools will run on github actions and you will get error messages (by email) if they fail. Feature branches and Pull Requests are not merged into master before they build successfully!

clang-format

When clang-format fails during continuous integration, the build output on github actions will show you how you should format your files. In principle you can then make the appropriate changes and push another commit. However, it's much easier if you let clang-format make these changes for you:

  • Make sure you have a recent version of clang-format on your computer, the Continuous Integration pipeline uses clang-format-10. For best results you use the same version.
  • To reformat a single file run clang-format -i <path to file>
  • To check if all your files have been formatted correctly, run the bash script run_clang_format.sh from the root folder of this repository (on linux). This will apply the style that is specified in the file .clang-format.

There exist integrations for many popular editors that apply clang-format for instance whenever you save a file. This is a huge help since you don't have to think about formatting at all anymore :)

clang-tidy

clang-tidy is a lint-tool that points out common programming mistakes. When it fails on github actions you will also see a list of errors that must be corrected. You can either correct these mistakes based on the output of the github action jobs or run clang-tidy locally on your machine before every commit (recommended):

  • Make sure you have a recent version of clang-tidy on your computer. The continuous integration pipeline uses clang-tidy-10, it's strongly suggested to install the same version on your machine so that you get the same error messages.
  • change into a CMake build directory and make sure that the CMake variable CMAKE_EXPORT_COMPILE_COMMANDS is set: cmake -DCMAKE_EXPORT_COMPILE_COMMANDS=On .. This will create the file compile_commands.json.
  • To run clang-tidy only over one specific source file (header files don't work because they cannot be compiled), run clang-tidy-8 -p . <path to source file> from the cmake build folder.
  • To run clang-tidy over the whole repository, execute the bash script run_clang_tidy.sh from the cmake build folder (linux only).
  • Correct the errors that clang-tidy complains about. Note that clang-tidy can fix some problems automatically, see the -fix command line option.

2. Testing

LehrFEM++ makes extensive use of the Google's C++ test framework. Adding a test is very simple and we encourage you to submit/change tests whenever you submit a pull request:

  • If you add new functionality, write new unit/integration test(s) that test this functionality.
  • If you fix a bug, write a unit/integration test that tests this bug.

2.1. How to add a test

Lets say you want to add a new test for the class lf::geometry::QuadO1. Then you have to do two things:

  1. Check if the file lib/lf/geometry/test/quad_o1_tests.cc already exists. If not, create it and register it in lib/lf/geometry/test/CMakeLists.txt.
  2. Write your tests into the quad_o1_tests.cc file. Since you will be testing the class lf::geometry::QuadO1 you should put your tests into a Google Test Case/Suite with the name QuadO1. The tests should be defined inside the namespace lf::geometry::test. I.e. your test file should look similar to this:
#include <gtest/gtest.h>
// more includes
namespace lf::geometry::test {
TEST(QuadO1, Jacobian) {
  // test for QuadO1::Jacobian() here...
}
}

Implicit in the above description are the following conventions:

  • There is one C++ source test file for each class that should be tested. If the class x::y::Z lies in module/namespace x::y then the C++ source file that contains the tests should be placed at lib/x/y/test/z_tests.cc.
  • The source file should use the lower_underscore naming convention and end with _tests.cc.
  • There is one Google Test Case/Suite for each class that is to be tested. For the class x::y::Z, the test case should be named Z and all tests should be defined in the namespace x::y::test.

See the Google Test Primer for more information about writing tests.

2.2. Reusing tests

Since LehrFEM++ relies heavily on abstract interface classes, it is often possible to write test routines that can be used to test all/most classes that implement a given interface. Lets say we want to write a test routine that checks the output of the lf::geometry::Geometry::Jacobian() method by numeric differentiation. Then the procedure is as follows:

  1. Create the file lib/lf/geometry/test/check_jacobian.h and declare a function that accepts any Geometry object and tests the jacobians:
#ifndef <__include_guard_here>
#define <__include_guard_here>

#include <gtest/gtest.h>
#include <lf/geometry/geometry.h>

namespace lf::geometry::test {
void checkJacobian(const Geometry& geometry);
}
#endif
  1. Because checkJacobian() is a normal function (and not a function template), we can define it in a separate C++ source file (which saves compilation time). We place it at lib/lf/geometry/test/check_jacobian.cc:
#include "check_jacobian.h"

namespace lf::geometry::test {
void checkJacobian(const Geometry& geometry) {
  // Check implementation here with automatic differentiation.
}
}
  1. Register check_jacobian.h and check_jacobian.cc in lib/lf/geometry/test/CMakeLists.txt
  2. Create a corresponding test for each class where we want to test the jacobian. E.g. we would add the following test to lib/lf/geometry/test/quad_o1.cc:
#include "check_jacobian.h"
namespace lf::geometry::test {
TEST(QuadO1, Jacobian) {
  QuadO1 geom = ...;
  checkJacobian(geom);
}
}

2.2.1. Test Modules

Sometimes it makes sense to make check routines available to other modules/executables. Such routines should be put into test_utils modules. These test_utils modules can then be imported by other test modules/executables.

An example of this is the function lf::mesh::test_utils::testEntityIndexing(const Mesh& mesh) which takes a mesh and checks that the indices of the entities are all consecutive. Since this method works with any Mesh object, it doesn't make sense to put it e.g. into lf::mesh::hybrid2d::test since the tests in lf::mesh::hybrid3d::test (doesn't exist yet) probably also want to use it.

The lf::mesh::hybrid2d::test executable can then import the lf::mesh::test_utils module and call the method testEntityIndexing.

3. Logging / Debug output

Sometimes it is necessary to include code in LehrFEM++ that prints information to the console for debugging purposes. The general rule is as follows:

  1. Usually you remove this code again before you make the PR.
  2. If you think that the code will also be useful for other users, you can either move it into a PrintInfo() method or write it to a logger. In this case please make sure that:
    • You have read and understood the corresponding section of the LehrFEM++ manual
    • You log only at level Trace or Debug (higher log levels only if every possible user should know about this)
    • You use the macro SPDLOG_LOGGER_TRACE or SPDLOG_LOGGER_DEBUG for the logging. These statements are then removed during Release mode compilation.
    • If you need to create a new logger:
      • If it is for a free function, create it in the same namespace as the free function.
      • If it is for a member function, add a public static member name logger to the class.
      • In both cases, initialize the logger with a call to lf::base::InitLogger("<fully qualified name of the logger here>").

1: Strictly speaking, this is an extension of the Google Coding style guide.

Clone this wiki locally