Some cleanup of unused build-time configuration options

.gitignore

@@ -4,6 +4,7 @@ googletest*
 tidy_results*.txt
 bin/*
 docs/cached_mtimes.json
+src/cholla_config.h
 
 # Compiled source #
 ###################

Makefile

@@ -77,8 +77,12 @@ GPUFLAGS          += $(GPUFLAGS_$(BUILD))
 
 #-- Add flags and libraries as needed
 
-CXXFLAGS += $(DFLAGS) -Isrc
-GPUFLAGS += $(DFLAGS) -Isrc
+# by passing `-include cholla_config.h` to the compiler, the C preprocessor
+# acts as if the very first line of the source is `#include "cholla_config.h"`
+#
+# this is bad practice
+CXXFLAGS += -I./src -include cholla_config.h
+GPUFLAGS += -I./src -include cholla_config.h
 
 ifeq ($(findstring -DPARIS,$(DFLAGS)),-DPARIS)
   ifdef HIPCONFIG
@@ -172,8 +176,8 @@ EXEC := bin/cholla$(SUFFIX)
 
 # Get the git hash and setup macro to store a string of all the other macros so
 # that they can be written to the save files
-DFLAGS      += -DGIT_HASH='"$(shell git rev-parse --verify HEAD)"'
-MACRO_FLAGS := -DMACRO_FLAGS='"$(DFLAGS)"'
+DFLAGS      += -DGIT_HASH=$(shell git rev-parse --verify HEAD)
+MACRO_FLAGS := -DMACRO_FLAGS='$(DFLAGS)'
 DFLAGS      += $(MACRO_FLAGS)
 
 # Setup variables for clang-tidy
@@ -196,10 +200,25 @@ $(EXEC): prereq-build $(OBJS)
 	mkdir -p bin/ && $(LD) $(LDFLAGS) $(OBJS) -o $(EXEC) $(LIBS)
 	eval $(EXTRA_COMMANDS)
 
-%.o: %.cpp
+# here's a trick to ensure that src/cholla_config.h's recipe is rerun without declaring
+# src/cholla_config.h to be PHONY (we shouldn't do that since the recipe makes the file)
+# -> https://stackoverflow.com/a/60724811
+.PHONY: FORCE
+FORCE: ;
+
+# this is the generated file that holds all of the DFLAGS
+# -> even though this recipe is rerun every time make is invoked to build a target
+#    with a direct or indirect dependency on src/cholla_config.h, we only mutate
+#    src/cholla_config.h if the contents of the file changes
+src/cholla_config.h: src/cholla_config.h.in FORCE
+	tools/configure_file.py --clobber --input $< --output $@.tmp $(DFLAGS)
+	cmp $@.tmp $@ || mv $@.tmp $@
+	rm -rf $@.tmp
+
+%.o: %.cpp src/cholla_config.h
 	$(CXX) $(CXXFLAGS) -c $< -o $@
 
-%.o: %.cu
+%.o: %.cu src/cholla_config.h
 	$(GPUCXX) $(GPUFLAGS) -c $< -o $@
 
 .PHONY: clean, clobber, tidy, format
@@ -212,13 +231,13 @@ tidy:
 # - --warnings-as-errors=<string> Upgrade all warnings to error, good for CI
 	clang-tidy --verify-config
 	@echo -e
-	(time clang-tidy $(CLANG_TIDY_ARGS) $(CPPFILES_TIDY) -- $(DFLAGS) $(CXXFLAGS_CLANG_TIDY) $(LIBS_CLANG_TIDY)) > tidy_results_cpp_$(TYPE).log 2>&1 & \
-	(time clang-tidy $(CLANG_TIDY_ARGS) $(GPUFILES_TIDY) -- $(DFLAGS) $(GPUFLAGS_CLANG_TIDY) $(LIBS_CLANG_TIDY)) > tidy_results_gpu_$(TYPE).log 2>&1 & \
+	(time clang-tidy $(CLANG_TIDY_ARGS) $(CPPFILES_TIDY) -- $(CXXFLAGS_CLANG_TIDY) $(LIBS_CLANG_TIDY)) > tidy_results_cpp_$(TYPE).log 2>&1 & \
+	(time clang-tidy $(CLANG_TIDY_ARGS) $(GPUFILES_TIDY) -- $(GPUFLAGS_CLANG_TIDY) $(LIBS_CLANG_TIDY)) > tidy_results_gpu_$(TYPE).log 2>&1 & \
 	for i in 1 2; do wait -n; done
 	@echo -e "\nResults from clang-tidy are available in the 'tidy_results_cpp_$(TYPE).log' and 'tidy_results_gpu_$(TYPE).log' files."
 
 clean:
-	rm -f $(CLEAN_OBJS)
+	rm -f $(CLEAN_OBJS) src/cholla_config.h
 	rm -rf googletest
 	-find bin/ -type f -executable -name "cholla.*.$(MACHINE)*" -exec rm -f '{}' \;
 	-find src/ -type f -name "*.gcno" -delete

builds/make.host.tornado

@@ -15,6 +15,3 @@ FFTW_ROOT    = /home/bruno/apps/fftw-3.3.5
 PFFT_ROOT    = /home/bruno/apps/pfft-1.0.8-alpha
 GRACKLE_ROOT = /home/bruno/code/grackle
 
-POISSON_SOLVER = -DCUFFT
-
-

builds/make.inc.template

@@ -1,4 +1,3 @@
-#POISSON_SOLVER ?= -DPFFT
 #DFLAGS += $(POISSON_SOLVER)
 
 #To use MPI, DFLAGS must include -DMPI_CHOLLA
@@ -49,9 +48,6 @@ DFLAGS += -DTEMPERATURE_FLOOR
 # Average Slow cell when the cell delta_t is very small
 #DFLAGS += -DAVERAGE_SLOW_CELLS
 
-# Allocate GPU memory every timestep
-#DFLAGS += -DDYNAMIC_GPU_ALLOC
-
 # Set the cooling function
 #DFLAGS += -DCOOLING_GPU
 #DFLAGS += -DCLOUDY_COOL
@@ -69,8 +65,6 @@ DFLAGS += -DCPU_TIME
 # Include FFT gravity
 #DFLAGS += -DGRAVITY
 #DFLAGS += -DGRAVITY_LONG_INTS
-#DFLAGS += -DCOUPLE_GRAVITATIONAL_WORK
-#DFLAGS += -DCOUPLE_DELTA_E_KINETIC
 #DFLAGS += -DOUTPUT_POTENTIAL
 #DFLAGS += -DGRAVITY_5_POINTS_GRADIENT
 

builds/make.type.basic_scalar

@@ -28,6 +28,6 @@ DFLAGS += -DBASIC_SCALAR
 #DFLAGS    += -DCPU_TIME
 
 # Select output format
-# Can also add -DSLICES and -DPROJECTIONS
+# Can also add -DSLICES and -DPROJECTION
 OUTPUT    ?=  -DOUTPUT -DHDF5
 DFLAGS    += $(OUTPUT)

builds/make.type.hydro

@@ -27,7 +27,7 @@ DFLAGS    += -DTEMPERATURE_FLOOR
 #DFLAGS    += -DCPU_TIME
 
 # Select output format
-# Can also add -DSLICES and -DPROJECTIONS
+# Can also add -DSLICES and -DPROJECTION
 OUTPUT    ?=  -DOUTPUT -DHDF5
 DFLAGS    += $(OUTPUT)
 

builds/make.type.particles

@@ -28,7 +28,14 @@ DFLAGS += -DSINGLE_PARTICLE_MASS
 
 
 #If the particles are solved on the CPU, use OpenMP for better performance
-DFLAGS += -DPARALLEL_OMP
 #-- OMP_NUM_THREADS should be set in make.host.*
-DFLAGS += -DN_OMP_THREADS=$(OMP_NUM_THREADS)
+# (take some care to ensure the variable isn't set twicee)
+ifneq ($(findstring -DPARALLEL_OMP,$(DFLAGS)),-DPARALLEL_OMP)
+  DFLAGS += -DPARALLEL_OMP
+endif
 
+#-- OMP_NUM_THREADS should be set in make.host.*
+# (take some care to ensure the variable isn't set twicee)
+ifneq ($(findstring -DN_OMP_THREADS,$(DFLAGS)),-DN_OMP_THREADS)
+  DFLAGS += -DN_OMP_THREADS=$(OMP_NUM_THREADS)
+endif

builds/make.type.rot_proj

@@ -23,7 +23,7 @@ DFLAGS    += -DTEMPERATURE_FLOOR
 #DFLAGS    += -DCPU_TIME
 
 # Select output format
-# Can also add -DSLICES and -DPROJECTIONS
+# Can also add -DSLICES and -DPROJECTION
 OUTPUT    ?=  -DOUTPUT -DHDF5
 DFLAGS    += $(OUTPUT)
 

builds/make.type.starblast

@@ -22,7 +22,6 @@ DFLAGS += -DANALYSIS
 #DFLAGS += -DSTATIC_GRAV
 
 #DFLAGS    += -DOUTPUT_ALWAYS
-DFLAGS    += -DCUDA
 DFLAGS    += -DMPI_CHOLLA 
 DFLAGS    += -DPRECISION=2
 DFLAGS    += -DPLMC
@@ -44,5 +43,3 @@ DFLAGS    += $(MPI_GPU)
 
 #DFLAGS += -DPARALLEL_OMP
 DFLAGS += -DN_OMP_THREADS=$(OMP_NUM_THREADS)
-
-DFLAGS += -DCUDA_ERROR_CHECK

builds/make.type.static_grav

@@ -25,6 +25,6 @@ DFLAGS    += -DSTATIC_GRAV
 #DFLAGS    += -DCPU_TIME
 
 # Select output format
-# Can also add -DSLICES and -DPROJECTIONS
+# Can also add -DSLICES and -DPROJECTION
 OUTPUT    ?=  -DOUTPUT -DHDF5
 DFLAGS    += $(OUTPUT)

docs/sphinx/CompilingCholla.md

@@ -18,4 +18,4 @@ If you don't know your host name, you can type "hostname --fqdn" to determine it
 The top-level cholla directory includes a Makefile, which is not designed to be edited directly. If no make type is specified when compiling, the Makefile assumes you want to build for basic hydro.
 
 ## make.type
-If you want to run using a preset collection of Makefile flags, you can just type "make" in the top-level directory, which will build a hydro-only version of cholla and put the executable in the "bin" directory. If you would like to build for one of the other presets, you can use, for example "make TYPE=gravity", which includes all the necessary flags for the default hydro build, plus flags needed for the FFT gravity solver. Take a look at the other make.type.* files for more examples. You can also define your own make.type. file to build your favorite version of Cholla. The docs page [Makefile Parameters](https://cholla.readthedocs.io/en/latest/MakefileParameters.html) contains most of the available flags, along with brief descriptions.
+If you want to run using a preset collection of Makefile flags, you can just type "make" in the top-level directory, which will build a hydro-only version of cholla and put the executable in the "bin" directory. If you would like to build for one of the other presets, you can use, for example "make TYPE=gravity", which includes all the necessary flags for the default hydro build, plus flags needed for the FFT gravity solver. Take a look at the other make.type.* files for more examples. You can also define your own make.type. file to build your favorite version of Cholla. The docs page {ref}`makefile-parameters` contains most of the available flags, along with brief descriptions.

docs/sphinx/Development/build-time-config.md

@@ -0,0 +1,52 @@
+(dev-build-time-config)=
+# Build-Time Configuration
+
+This page provides a developer guide for build-time configuration.
+The full list of options can be found {ref}`here <makefile-parameters>`
+
+## How Config Parameters are Communicated to the Source Code
+
+At the time of writing, the build-system tracks configuration options in the ``DFLAGS``  variable.
+In more detail:
+- as ``make`` parses the makefiles it appends a token encoding each configuration variable's to ``DFLAGS``
+- a token of defining a variable, ``<var>``, is specified as either ``-D<var>`` or ``-D<var>=<value>``.
+  The first format defines a variable without assigning a value while the second variable assocciates the value ``<value>``.
+
+Originally, the build system made the source code aware of the configuration options by passing all the tokens in ``DFLAGS`` directly as arguments to the compiler.
+More recently, we have adopted a new strategy.
+
+At present, the build-system uses the {ref}`configure_file.py tool <about-configure-file>` to communicate the configuration parameters.
+Specifically, the build system passes all the tokens in ``DFLAGS`` as arguments to ``configure_file.py`` to generate a file called ``cholla_config.h`` from {repository-file}`src/cholla_config.h.in`.
+Then the compiler is passed the option ``-include cholla_config.h``, which tells the compiler to act like ``#include "cholla_config.h"`` is the first line of every source file.
+
+:::{important}
+The use of ``-include cholla_config.h`` is something of a short-term hack.
+There are some concerns that this may not work on all C++ compilers (it was decided that since it works for ``g++`` and ``clang++``, it's ok for now).
+
+The more robust long-term plan is to make every source/header file that needs a compile-time configuration parameter directly include the ``cholla_config.h`` header or include the ``global/global.h`` header (which itself includes ``cholla_config.h``).
+Unfortunately, this is not immediately viable because both:
+1. the checks of configuration options are currently ubiquitous (I think almost every single source/header has at least one check).
+2. Just about every single check checks whether a configuration option is defined or not.
+   Thus, there's a degeneracy between "forgetting to include the ``cholla_config.h`` header and choosing not to enable a feature.
+:::
+
+## How to add a new build-time configuration parameter
+
+:::{important}
+Before doing anything, ask yourself "can this be a runtime parameter?"
+If the answer is "yes" (and it usually is), then sooner or later somebody is going to have to go to the trouble of converting it to a runtime parameter.
+
+Your code contribution may not be merged until you convert it to a runtime parameter.
+:::
+
+To add a new parameter, you need to both:
+1. add the option to a Makefile
+2. add an appropriate line into {repository-file}`src/cholla_config.h.in`
+
+If you forget to do the second step, then the ``configure_file.py`` tool will fail (and the entire build will fail).
+This behavior is a FEATURE -- it makes the build fail if an option is mispelled when modifying a Makefile.
+
+### Best Practice
+
+If you find yourself adding a new configuration parameter, you should STRONGLY prefer to configure things so that the configuration parameter is ALWAYS defined and the only thing that changes is its associated value (e.g. 0 or 1).
+This generally produces more robust code (and is an important step towards direct inclusion of ``cholla_config.h`` in all files).

docs/sphinx/Development/configure_file.md

@@ -0,0 +1,82 @@
+(about-configure-file)=
+# configure_file.py tool
+
+The {repository-file}`tools/configure_file.py` script is a tool invoked by the build system.
+
+The tool is intended to be used to generate files that are treated as part of the codebase and that hold configuration values set by the build-system.
+In short, it acts as an alternative to repeatedly passing long lists of configuration options to the compiler for every source file.
+
+It's instructive to consider the structure of a typical invocation of the tool:
+
+```sh
+configure_file.py --input=<template-file> --output=<output> [-D<var>[=<value>]]...
+```
+
+The snippet illustrates that the tool recieves 3 kinds of information:
+1. The input to the template file read by the tool
+2. The path to the output file to be produced by the tool
+3. A variable number of options for specifying configuration variables.
+   ``-D<var>`` defines a configuration variable ``<var>`` without a value while ``-D<var>=<value>`` associates the variable with the value ``<value>`` 
+
+> Aside: a complete overview of all available options is provided {ref}`later <detailed-configure-file-help>`
+
+Essentially, the tool reads template file, performs transformations based upon configuration variables and write out the result.
+
+Analogous functionality is implemented by various build systems such as [autotools](https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Setting-Output-Variables.html), [CMake](https://cmake.org/cmake/help/latest/command/configure_file.html), and [Meson](https://mesonbuild.com/Configuration.html).
+**This tool is closely modelled after the ``confiure_file`` commands provided by CMake and Meson.
+
+## Transformations
+
+The tool performs 2 type of tranformations.
+
+The first transformation type is straightforward **variable substitution**.
+The tool replaces tokens enclosed in a pair of ``@`` symbols with the value of the configuration variable matching the token.
+The tool aborts with an error if the configuration variable isn't defined or doesn't have an associated value.
+Let's consider 2 examples:
+1. Consider a template file with the line
+   ```c++
+   const char* git_hash = "@GIT_HASH@";
+   ```
+   If the tool is passed `-DGIT_HASH=f4ac1200c2fa21f72a9f160f38f5924f440d19508` the corresponding line in the output file will read:
+   ```c++
+   const char* git_hash = "f4ac1200c2fa21f72a9f160f38f5924f440d19508";
+   ```
+2. Consider a template file with the line
+   ```c++
+   #define PRECISION @PRECISION@
+   ```
+   If the tool is passed `-DPRECISION=2` the corresponding line in the output file will read:
+   ```c++
+   #define PRECISION 2
+   ```
+
+We refer the second type of transformation as a **define-transfrom**.
+This occurs for a line in the input file of the form:
+```c++
+#configurefile_define CONFVAR
+```
+where ``CONFVAR`` is a placehold the name of any configuration variable.
+The corresponding line of the output file has 3 possible forms:
+1. If ``-DCONFVAR`` was passed to the tool, then the output line is
+   ```c++
+   #define CONFVAR
+   ```
+2. If ``-DCONFVAR=42`` was passed to the tool, then the output line is:
+   ```c++
+   #define CONFVAR 42
+   ```
+   In this scenario, 42 is intended as an arbitrary choice for the associated value
+3. If ``CONFVAR`` is not the name of a configuration variable, the output becomes
+   ```c++
+   /* #undef CONFVAR */
+   ```
+
+:::{note}
+The analogous transformations in CMake and Meson replace lines that start with ``#cmakedefine`` and ``#mesondefine``.
+:::
+
+(detailed-configure-file-help)=
+## Detailed Help Output
+
+:::{include-cli-help} ../../../tools/configure_file.py
+:::

docs/sphinx/Development/index.md

@@ -7,4 +7,6 @@ GettingStarted.md
 writing-docs.md
 StyleGuide.md
 cholla_utils.md
+configure_file.md
+build-time-config.md
 :::

docs/sphinx/MakefileParameters.md

@@ -1,7 +1,10 @@
+(makefile-parameters)=
 # Makefile Parameters
 
 Most of the configuration options in Cholla currently require turning on or off flags in a make.type file. This page contains most of the available options, along with a brief summary of their meanings.
 
+Developer Documentation about how these parameters are used and how to add a new parameter are provided on {ref}`this linked pag <dev-build-time-config>`.
+
 ## General options
 
 * **DISABLE_GPU_ERROR_CHECKING**: Turns off optional GPU error checking. GPU error checking is on by default with a device sync that may impact performance. Passing this macro flag disables GPU error checking altogether and may improve performance.