Contributing
If you have any trouble with the project, or are interested in participating, please contact us by creating an issue on the github repository, or submit a pull request!
Pull request protocol
There is a pull reuqest template that will be auto-populated when you submit a pull request. A pull request should have a summary of changes. You should also add tests for bugs fixed or new features you add.
We have a changelog file, CHANGELOG.md. After creating your pull
request, add the relevant change and a link to the PR in the
changelog.
Before a pull request will be merged, the code should be formatted. We
use clang-format for this, pinned to version 12. You can automatically
trigger clang-format in two ways: first you can run the script
utils/scripts/format.sh; second you can type make
format_singularity after configuring the code with clang-format
discoverable by cmake.
Several sets of tests are triggered on a pull request: a static format check, a docs buld, and unit tests of analytic models and the stellar collapse model. These are run through github’s CPU infrastructure. We have a second set of tests run on a wider set of architectures that also access the Sesame library, which we are not able to make public.
The docs are built but not deployed on PRs from forks, and the internal tests will not be run automatically. So when the code is ready for merge, you must ask a project maintainer to trigger the remaining tests for you.
Expectations for code review
From the perspective of the contributor
Code review is an integral part of the development process
for singularity-eos. You can expect at least one, perhaps many,
core developers to read your code and offer suggestions.
You should treat this much like scientific or academic peer review.
You should listen to suggestions but also feel entitled to push back
if you believe the suggestions or comments are incorrect or
are requesting too much effort.
Reviewers may offer conflicting advice, if this is the case, it’s an opportunity to open a discussion and communally arrive at a good approach. You should feel empowered to argue for which of the conflicting solutions you prefer or to suggest a compromise. If you don’t feel strongly, that’s fine too, but it’s best to say so to keep the lines of communication open.
Big contributions may be difficult to review in one piece and you may be requested to split your pull request into two or more separate contributions. You may also receive many “nitpicky” comments about code style or structure. These comments help keep a broad codebase, with many contributors uniform in style and maintainable with consistent expectations accross the code base. While there is no formal style guide for now, the regular contributors have a sense for the broad style of the project. You should take these stylistic and “nitpicky” suggestions seriously, but you should also feel free to push back.
As with any creative endeavor, we put a lot of ourselves into our code. It can be painful to receive criticism on your contribution and easy to take it personally. While you should resist the urge to take offense, it is also partly code reviewer’s responsiblity to create a constructive environment, as discussed below.
Expectations of code reviewers
A good code review builds a contribution up, rather than tearing it down. Here are a few rules to keep code reviews constructive and congenial:
You should take the time needed to review a contribution and offer meaningful advice. Unless a contribution is very small, limit the times you simply click “approve” with a “looks good to me.”
You should keep your comments constructive. For example, rather than saying “this pattern is bad,” try saying “at this point, you may want to try this other pattern.”
Avoid language that can be misconstrued, even if it’s common notation in the commnunity. For example, avoid phrases like “code smell.”
Explain why you make a suggestion. In addition to saying “try X instead of Y” explain why you like pattern X more than pattern Y.
A contributor may push back on your suggestion. Be open to the possibility that you’re either asking too much or are incorrect in this instance. Code review is an opportunity for everyone to learn.
Don’t just highlight what you don’t like. Also highlight the parts of the pull request you do like and thank the contributor for their effort.
General principle for everyone
It’s hard to convey tone in text correspondance. Try to read what others write favorably and try to write in such a way that your tone can’t be mis-interpreted as malicious.
Interwoven Dependencies
singularity-eos depends on several other open-source, Los Alamos
maintained, projects. In particular, spiner and
ports-of-call. If you have issues with these projects, ideally
submit issues on the relevant github pages. However, if you can’t
figure out where an issue belongs, no big deal. Submit where you can
and we’ll engage with you to figure out how to proceed.
Note
There are scheduled workflows triggered by GitHub actions that will
automatically check spiner and ports-of-call for Spack updates. If
detected, the GitHub action bot will create a PR with the necessary changes.
Process for adding a new EOS
The basic process for adding a new EOS can be summarized as
Create a new header file for your EOS
Add the EOS to the
full_eos_listlist of EOS ineos.hppCreate tests for your EOS
Create a Fortran interface to initialize your EOS into an array of EOS
In addition to these main steps, there are a couple more that are required if you would like your EOS to work with our fortran interface, which will be discussed below.
Step 1: Create a new header file for your EOS
In general, the best practice is to simply copy an existing EOS file and modify it for the new EOS. However, there are some subtleties here that are important.
Parameters for the EOS can be initialzed via an initializer list, and additional parameter checking can be done in the constructor.
Any EOS must have a set of member functions that conform to the general EOS API. In essence, these functions are defined by the
Variantclass as avisiton the underlying member of the specific EOS type contained in thevariant. If a new EOS doesn’t have an appropriate member, a compilation error will be thrown when theEOStype is used to instantiate an instance of the new EOS. This will be discussed more in the testing section.You may find it useful to define other functions that are specific to that EOS but won’t be available to the general
EOStype. These can be internal checking functions or common calculations that need to be performed for multiple types of lookups.An analytic EOS needs to be “trivially copiable” in order to use the standard
GetOnDevice()function that we use for analytic EOS. In general, analytic EOS should only need parameters whose size is known at compile time, so this should be fairly straight-forward. Any EOS that needs dynamic memory (e.g. a tabular EOS) will need more effort in order to ensure that memory is copied correctly over to the device.
Step 2: Add the EOS to the full_eos_list list of EOS in eos.hpp
As was mentioned previously, we use the Variant class and a visit
pattern to achieve compile-time polymorphism on a closed set of types. For
convenience, we provide this closed set in the eos.hpp file through the
type list, full_eos_list.
For most new EOS, you can simply add the EOS to the full_eos_list and this
will enable all of the modifiers (with certain exceptions) to instantly work
with your EOS. This would effectively look like
static constexpr const auto full_eos_list =
tl<IdealGas, Gruneisen, JWL, DavisReactants, DavisProducts, MyNewEOS
Note the lack of a trailing comma or closing angle bracket.
If your EOS introduces new dependencies to singularity-eos, then you will
need to create a new flag that enables these dependencies. Then you will need to
wrap the inclusion of your EOS in the full_eos_list with an appropriate
#ifdef <my_dependency_flag> preprocessor directive. For example, the EOSPAC
EOS needs the SINGULARITY_USE_EOSPAC flag so the inclusion in the list is
wrapped with #ifdef SINGULARITY_USE_EOSPAC. This might look something like
static constexpr const auto full_eos_list =
tl<IdealGas, Gruneisen, JWL, DavisReactants, DavisProducts
//
// ...the other EOS that have dependencies
//
#ifdef MY_NEW_DEP_FLAG
,
MyNewEOS
#endif
>{};
Note the placement of commas and angle brackets. This example excludes
Step 3: Create tests for your EOS
Important: this is a sublty that highlights the importance of unit tests!
Since our library is header only, the unit tests are often the only place where
a specific EOS may be instantiated when singularity-eos is compiled. Unit
tests _must_ make use of the EOS type, i.e.
#include <singularity-eos/eos/eos.hpp>
EOS my_eos = my_new_eos(parameter1, parameter2, ...)
in order to properly test the functionality of a new EOS. Simply using the new class as the type such as
#include <singularity-eos/eos/eos.hpp>
auto my_eos = my_new_eos(parameter1, parameter2, ...)
won’t ensure that the new EOS is working correctly in singularity with the
static polymorphism of the EOS type.
You may wish to also design tests that operate on member functions or member
data that is particular to the EOS you have developed, and only for those
specific tests should you instantiate an object whose type is your specific
EOS. Otherwise, use the EOS object.
If you wish to test error handling in your EOS, you may use the macro
REQUIRE_MAYBE_THROWS, which is defined in the eos_unit_test_helpers.hpp
header file. This macro will check if your code throws an exception if
compiled for CPU only and otherwise is a no-op. This is intended to combine with
the PORTABLE_THROW_OR_ABORT` macro defined in ``ports-of-call.
Step 4: Fortran interface
At this point your new EOS should be usable to any host code written in C++. To allow the EOS to work with Fortran, an initializer wrapper function needs to be defined and interfaced with Fortran.
First, the C++ intialization function needs to be named soas to avoid namespace
conflicts. We typically name the initialization functions init_sg_<EOSName>.
For example, the function for initialing an ideal gas looks like
int init_sg_IdealGas(const int matindex, EOS *eos, const double gm1,
const double Cv, int const *const enabled,
double *const vals) {
assert(matindex >= 0);
EOS eosi = SGAPPLYMODSIMPLE(IdealGas(gm1, Cv));
if (enabled[3] == 1) {
singularity::pAlpha2BilinearRampParams(eosi, vals[2], vals[3], vals[4], vals[2],
vals[3], vals[4], vals[5]);
}
EOS eos_ = SGAPPLYMOD(IdealGas(gm1, Cv));
eos[matindex] = eos_.GetOnDevice();
return 0;
}
Here the *eos is a pointer to a container of EOS objects and the
matindex integer indicates the index at which this EOS will reside in that
container. The gm1 and Cv inputs are all of the required parameters to
initialize the EOS, while the enabled and vals variables are used by
the SGAPPLYMOD and SGAPPLYMODSIMPLE macros to apply specific modifiers
to the EOS. The return value of the function is an integer error code that may
or may not be relevant to all EOS.
We also overload the initialization function to make the enabled and
vals variables effectively optional.
int init_sg_IdealGas(const int matindex, EOS *eos, const double gm1,
const double Cv) {
return init_sg_IdealGas(matindex, eos, gm1, Cv, def_en, def_v);
}
Finally the fortran side, we then define a fortran interface to the C++ initialization function,
interface
integer(kind=c_int) function &
init_sg_IdealGas(matindex, eos, gm1, Cv, sg_mods_enabled, &
sg_mods_values) &
bind(C, name='init_sg_IdealGas')
import
integer(c_int), value, intent(in) :: matindex
type(c_ptr), value, intent(in) :: eos
real(kind=c_double), value, intent(in) :: gm1, Cv
type(c_ptr), value, intent(in) :: sg_mods_enabled, sg_mods_values
end function init_sg_IdealGas
end interface
and a fortran wrapper function to call the C++ function:
integer function init_sg_IdealGas_f(matindex, eos, gm1, Cv, &
sg_mods_enabled, sg_mods_values) &
result(err)
integer(c_int), value, intent(in) :: matindex
type(sg_eos_ary_t), intent(in) :: eos
real(kind=8), value, intent(in) :: gm1, Cv
integer(kind=c_int), dimension(:), target, intent(inout) :: sg_mods_enabled
real(kind=8), dimension(:), target, intent(inout) :: sg_mods_values
err = init_sg_IdealGas(matindex-1, eos%ptr, gm1, Cv, &
c_loc(sg_mods_enabled), c_loc(sg_mods_values))
end function init_sg_IdealGas_f
Note that the eos variable of type sg_eos_ary_t is just a simple wrapper
for the C pointer to the actual EOS object.
A Note on the EOS Builder
The EOS Builder is a tool that eliminates the need for chaining together an EOS with a series of modifiers by instead specifing the parameters and modifications in one function. This convenience comes at the cost of added development complexity though, and so we do not require a new EOS to be available for the EOS Builder.
At a basic level though, the EOS needs to be declared in the EOSType enum
and logic needs to be added to initialze the EOS parameters. More effort may be
needed to make the EOS compatible with modifiers and we point the interested
contributor to the existing EOS as examples.