Merge pull request #139 from mabruzzo/deprecating-functions

Deprecating functions

doc/source/Integration.rst

@@ -222,6 +222,8 @@ of using comoving units, see the `cosmological unit system
 <https://github.com/enzo-project/enzo-dev/blob/main/src/enzo/CosmologyGetUnits.C>`__
 in the `Enzo <http://enzo-project.org/>`_ code.
 
+.. _setup_data-storage:
+
 Chemistry Data
 --------------
 
@@ -713,7 +715,7 @@ Clearing the memory
 
 .. code-block:: c++
 
-  _free_chemistry_data(my_grackle_data, &grackle_rates);
+  free_chemistry_data();
 
 Grackle is using global structures and therefore the global structure ``grackle_rates`` needs also to be released.
 

doc/source/Reference.rst

@@ -41,6 +41,14 @@ Primary Functions
    :rtype: int
    :returns: 1 (success) or 0 (failure)
 
+.. c:function:: int free_chemistry_data ();
+
+   Deallocates all data allocations made during the call to :c:func:`initialize_chemistry_data`.
+   Issues may arise if the global ``grackle_data`` data structure was mutated between the call to :c:func:`initialize_chemistry_data` and the call to this function.
+
+   :rtype: int
+   :returns: 1 (success) or 0 (failure)
+
 .. c:function:: void set_velocity_units(code_units *my_units);
 
    Sets the :c:data:`velocity_units` value of the input ``my_units``
@@ -162,16 +170,121 @@ Local Functions
 
 These can be used to create explicitly thread-safe code or to call
 the various functions with different parameter values within a
-single code. The :c:data:`chemistry_data` and
-:c:data:`chemistry_data_storage` structs should be setup using the
-initialization functions discussed in :ref:`internal_functions`.
+single code.
+
+.. _local_setup_data-storage:
+
+Initializing/Configuring Chemistry Parameters and Storage
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+
+..
+   COMMENT BLOCK
+   This section helps simplify the description of the local function
+   (it provides a place for us to point people to when we talk about
+   expectations for how the arguments are initialized). Furthermore,
+   there currently isn't any other place where we currently provide
+   this sort of overview of using the local functions.
+
+   In reality this content probably fits better on the page about
+   "Adding Grackle to Your Simulation Code". However, this duplicates
+   a bunch of content related to using the global Primary functions.
+   Maybe we could use the sphinx_tabs extension...
+
+
+The approach for initializing/configuring the :c:data:`chemistry_data` and :c:data:`chemistry_data_storage` structs for use with the :ref:`local_functions` differs to some degree from the :ref:`previously described approach <setup_data-storage>` that is used with :ref:`primary_functions`.
+
+We highlight the steps down below.
+For the sake of argument let's imagine that the user is storing the chemistry data in a variable called ``my_chemistry``.
+
+1. First, the user should allocate ``my_chemistry`` and initialize the stored parameters with :c:func:`local_initialize_chemistry_parameters`.
+
+   .. code-block:: c++
+
+      chemistry_data *my_chemistry = new chemistry_data;
+      if (local_initialize_chemistry_parameters(my_chemistry) == 0) {
+         fprintf(stderr, "Error in local_initialize_chemistry_parameters.\n");
+      }
+
+2. Next, the user can configure the stored parameters.
+   They can do this by directly modifying the stored parameters (e.g. ``my_chemistry->use_grackle = 1``) or by using the :ref:`dynamic-api`.
+
+After the user has finished initializing ``my_chemistry``, and has configured an instance of :c:data:`code_units` (more detail provided :ref:`here <code-units>`), they can initialize an instance of :c:data:`chemistry_data_storage` with 
+:c:func:`local_initialize_chemistry_data`:
+
+.. code-block:: c++
+
+  chemistry_data_storage* my_rates = new chemistry_data_storage;
+  if (local_initialize_chemistry_data(my_chemistry, my_rates, &my_units) == 0) {
+    fprintf(stderr, "Error in local_initialize_chemistry_data.\n");
+    return 0;
+  }
+
+Configuration/Cleanup Functions
++++++++++++++++++++++++++++++++
+
+.. c:function:: int local_initialize_chemistry_parameters \
+                (chemistry_data *my_chemistry);
+
+   Initializes the parameters stored in the :c:type:`chemistry_data` data structure to their default values.
+   This should be called before run-time parameters are set.
+
+   This is the "local" counterpart to :c:func:`set_default_chemistry_parameters`.
+
+   :param chemistry_data \*my_chemistry: run-time parameters
+   :rtype: int
+   :returns: 1 (success) or 0 (failure)
+
+.. c:function:: int local_initialize_chemistry_data \
+                (chemistry_data *my_chemistry, \
+                chemistry_data_storage *my_rates, \
+                code_units *my_units);
+
+   Allocates storage for and initializes the values of all relevant chemistry and cooling rate data.
+   This data is stored within the provided :c:data:`chemistry_data_storage` struct.
+   This is the "local" counterpart to :c:func:`initialize_chemistry_data`.
+
+   This function should only be called after the user has finished configuring both ``my_chemistry`` and ``my_units``.
+   This function assumes that none of ``my_rates``'s members of pointer type hold valid memory addresses (i.e. where applicable, the function allocates fresh storage and makes no attempts to deallocate/reuse storage).
+
+   After calling this function, the user should avoid modifying any of the fields of ``my_chemistry``.
+   The user should also avoid modifying ``my_units`` in a way that modifies the internal cooling units (e.g. it's fine to mutate ``my_units``'s :c:var:`a_value` field when its :c:var:`comoving_coordinates` field is ``1``).
+
+   To deallocate any storage allocated by this function, use :c:func:`free_chemistry_data`.
+
+   :param chemistry_data \*my_chemistry: :ref:`fully configured <local_setup_data-storage>` run-time parameters
+   :param chemistry_data_storage \*my_rates: chemistry and cooling rate data structure
+   :param code_units \*my_units: code units conversions
+   :rtype: int
+   :returns: 1 (success) or 0 (failure)
+
+   .. note::
+      In addition to modifying the contents of ``my_rates``, this function may also mutate the values stored in ``my_chemistry`` to set them to "more sensible" values (based on other values stored in ``my_chemistry``).
+
+
+.. c:function:: int local_free_chemistry_data \
+                (chemistry_data *my_chemistry, \
+                chemistry_data_storage *my_rates);
+
+   Deallocates all data held by the members of ``my_rates`` allocated during its initialization in :c:func:`local_initialize_chemistry_data` (or :c:func:`initialize_chemistry_data`).
+   Issues may arise if ``my_chemistry`` was mutated between the initialization of ``my_rates`` and the call to this function.
+
+   This is the "local" counterpart to :c:func:`free_chemistry_data`.
+
+   :param chemistry_data \*my_chemistry: :ref:`fully configured <local_setup_data-storage>` run-time parameters
+   :param chemistry_data_storage \*my_rates: previously initialized chemistry and cooling rate data structure
+   :rtype: int
+   :returns: 1 (success) or 0 (failure)
+
+
+Chemistry Functions
++++++++++++++++++++
 
 .. c:function:: int local_solve_chemistry(chemistry_data *my_chemistry, chemistry_data_storage *my_rates, code_units *my_units, grackle_field_data *my_fields, double dt_value);
 
    Evolves the species densities and internal energies over a given timestep
    by solving the chemistry and cooling rate equations.
 
-   :param chemistry_data* my_chemistry: the structure returned by :c:func:`_set_default_chemistry_parameters`
+   :param chemistry_data* my_chemistry: :ref:`fully configured <local_setup_data-storage>` run-time parameters
    :param chemistry_data_storage* my_rates: chemistry and cooling rate data structure
    :param code_units* my_units: code units conversions
    :param grackle_field_data* my_fields: field data storage
@@ -183,7 +296,7 @@ initialization functions discussed in :ref:`internal_functions`.
 
    Calculates the instantaneous cooling time.
 
-   :param chemistry_data* my_chemistry: the structure returned by :c:func:`_set_default_chemistry_parameters`
+   :param chemistry_data* my_chemistry: :ref:`fully configured <local_setup_data-storage>` run-time parameters
    :param chemistry_data_storage* my_rates: chemistry and cooling rate data structure
    :param code_units* my_units: code units conversions
    :param grackle_field_data* my_fields: field data storage
@@ -197,7 +310,7 @@ initialization functions discussed in :ref:`internal_functions`.
    :c:data:`primordial_chemistry` >= 2 as the only thing that alters gamma from the single
    value is H\ :sub:`2`.
 
-   :param chemistry_data* my_chemistry: the structure returned by :c:func:`_set_default_chemistry_parameters`
+   :param chemistry_data* my_chemistry: :ref:`fully configured <local_setup_data-storage>` run-time parameters
    :param chemistry_data_storage* my_rates: chemistry and cooling rate data structure
    :param code_units* my_units: code units conversions
    :param grackle_field_data* my_fields: field data storage
@@ -209,7 +322,7 @@ initialization functions discussed in :ref:`internal_functions`.
 
    Calculates the gas pressure.
 
-   :param chemistry_data* my_chemistry: the structure returned by :c:func:`_set_default_chemistry_parameters`
+   :param chemistry_data* my_chemistry: :ref:`fully configured <local_setup_data-storage>` run-time parameters
    :param chemistry_data_storage* my_rates: chemistry and cooling rate data structure
    :param code_units* my_units: code units conversions
    :param grackle_field_data* my_fields: field data storage
@@ -221,7 +334,7 @@ initialization functions discussed in :ref:`internal_functions`.
 
    Calculates the gas temperature.
 
-   :param chemistry_data* my_chemistry: the structure returned by :c:func:`_set_default_chemistry_parameters`
+   :param chemistry_data* my_chemistry: :ref:`fully configured <local_setup_data-storage>` run-time parameters
    :param chemistry_data_storage* my_rates: chemistry and cooling rate data structure
    :param code_units* my_units: code units conversions
    :param grackle_field_data* my_fields: field data storage
@@ -233,7 +346,7 @@ initialization functions discussed in :ref:`internal_functions`.
 
    Calculates the dust temperature.
 
-   :param chemistry_data* my_chemistry: the structure returned by :c:func:`_set_default_chemistry_parameters`
+   :param chemistry_data* my_chemistry: :ref:`fully configured <local_setup_data-storage>` run-time parameters
    :param chemistry_data_storage* my_rates: chemistry and cooling rate data structure
    :param code_units* my_units: code units conversions
    :param grackle_field_data* my_fields: field data storage
@@ -258,23 +371,23 @@ The following functions are used to provide dynamic access to members of the :c:
 
    Returns the pointer to the member of ``my_chemistry`` associated with ``param_name``.
 
-   :param chemistry_data* my_chemistry: the structure returned by :c:func:`_set_default_chemistry_parameters`
+   :param chemistry_data* my_chemistry: :ref:`fully configured <local_setup_data-storage>` run-time parameters
    :param const char* param_name: the name of the parameter to access.
    :rtype: int*
 
 .. c:function:: double* local_chemistry_data_access_double(chemistry_data *my_chemistry, const char* param_name);
 
    Returns the pointer to the member of ``my_chemistry`` associated with ``param_name``.
 
-   :param chemistry_data* my_chemistry: the structure returned by :c:func:`_set_default_chemistry_parameters`
+   :param chemistry_data* my_chemistry: :ref:`fully configured <local_setup_data-storage>` run-time parameters
    :param const char* param_name: the name of the parameter to access.
    :rtype: double*
 
 .. c:function:: char** local_chemistry_data_access_string(chemistry_data *my_chemistry, const char* param_name);
 
    Returns the pointer to the member of ``my_chemistry`` associated with ``param_name``.
 
-   :param chemistry_data* my_chemistry: the structure returned by :c:func:`_set_default_chemistry_parameters`
+   :param chemistry_data* my_chemistry: :ref:`fully configured <local_setup_data-storage>` run-time parameters
    :param const char* param_name: the name of the parameter to access.
    :rtype: char**
 
@@ -320,10 +433,14 @@ described here can be used in conjunction with the :ref:`local_functions`.
 
 .. c:function:: chemistry_data _set_default_chemistry_parameters(void);
 
+   This function has been deprecated and will be removed in versions
+   of Grackle later than 3.2.
+   Please use :c:func:`set_default_chemistry_parameters` or :c:func:`local_initialize_chemistry_parameters`.
+
    Initializes and returns :c:type:`chemistry_data` data structure. This must be
    called before run-time parameters can be set.
 
-   :returns: data structure containing all run-time parameters and all chemistry and cooling data arrays
+   :returns: data structure containing all run-time parameters
    :rtype: :c:type:`chemistry_data`
 
 .. c:function:: int _initialize_chemistry_data(chemistry_data *my_chemistry, chemistry_data_storage *my_rates, code_units *my_units);

src/clib/Make.config.assemble

@@ -193,7 +193,8 @@
     LDOUTPUT_FLAGS = $(ASSEMBLE_LDOUTPUT_FLAGS)
 
     DEFINES = $(MACH_DEFINES) \
-              $(ASSEMBLE_IO_DEFINES)
+              $(ASSEMBLE_IO_DEFINES) \
+              $(OTHER_DEFINES)
 
     INCLUDES = $(MACH_INCLUDES) \
                $(MAKEFILE_INCLUDES)   -I.

src/clib/Makefile

@@ -72,6 +72,13 @@ include $(MAKE_CONFIG_OVERRIDE)
 -include $(GRACKLE_DIR)/Make.mach.$(CONFIG_MACHINE)
 -include $(HOME)/.grackle/Make.mach.$(CONFIG_MACHINE)
 
+#-----------------------------------------------------------------------
+# OTHER_DEFINES is used to specify a macro indicating that legacy
+# functions defined inline in the main header file should be omitted
+# while compiling the library
+#-----------------------------------------------------------------------
+OTHER_DEFINES = -DOMIT_LEGACY_INTERNAL_GRACKLE_FUNC
+
 #-----------------------------------------------------------------------
 # Make.config.assemble takes the settings in the Make.config.settings
 # and Make.config.override, and generates the appropriate make variables

src/clib/grackle.h

@@ -31,13 +31,13 @@ double get_temperature_units(code_units *my_units);
 
 int set_default_chemistry_parameters(chemistry_data *my_grackle);
 
-chemistry_data _set_default_chemistry_parameters(void);
+int local_initialize_chemistry_parameters(chemistry_data *my_chemistry);
 
 int initialize_chemistry_data(code_units *my_units);
 
-int _initialize_chemistry_data(chemistry_data *my_chemistry, 
-                               chemistry_data_storage *my_rates,
-                               code_units *my_units);
+int local_initialize_chemistry_data(chemistry_data *my_chemistry, 
+                                    chemistry_data_storage *my_rates,
+                                    code_units *my_units);
 
 int* local_chemistry_data_access_int(chemistry_data* my_chemistry,
                                      const char* param_name);
@@ -183,8 +183,38 @@ int _calculate_temperature(chemistry_data *my_chemistry,
                            gr_float *e_density, gr_float *metal_density,
                            gr_float *temperature) __attribute__ ((deprecated));
 
-int _free_chemistry_data(chemistry_data *my_chemistry, chemistry_data_storage *my_rates);
+int free_chemistry_data(void);
+
+int local_free_chemistry_data(chemistry_data *my_chemistry, chemistry_data_storage *my_rates);
 
 grackle_version get_grackle_version(void);
 
+// Below, we conditionally define a handful of deprecated functions to maintain
+// backwards compatibility. These functions were deprecated because externally
+// visible identifiers that begin with an underscore invoke undefined behavior
+
+#ifndef OMIT_LEGACY_INTERNAL_GRACKLE_FUNC
+
+#warning "The legacy functions, _initialize_chemistry_data, _set_default_chemistry_parameters & _free_chemistry_data will be removed after version 3.2. To avoid defining these function (and this message) define the OMIT_LEGACY_INTERNAL_GRACKLE_FUNC macro."
+
+inline __attribute__((deprecated)) chemistry_data
+_set_default_chemistry_parameters(void) {
+  chemistry_data my_chemistry;
+  local_initialize_chemistry_parameters(&my_chemistry);
+  return my_chemistry;
+}
+
+inline __attribute__((deprecated)) int
+_initialize_chemistry_data(chemistry_data *my_chemistry,
+                           chemistry_data_storage *my_rates,
+                           code_units *my_units)
+{ return local_initialize_chemistry_data(my_chemistry, my_rates, my_units); }
+
+inline __attribute__((deprecated)) int
+_free_chemistry_data (chemistry_data *my_chemistry,
+                      chemistry_data_storage *my_rates)
+{ return local_free_chemistry_data(my_chemistry, my_rates); }
+
+#endif /* OMIT_LEGACY_INTERNAL_GRACKLE_FUNC */
+
 #endif

src/clib/initialize_chemistry_data.c

@@ -57,9 +57,9 @@ static void show_version(FILE *fp)
   fprintf (fp, "\n");
 }
 
-int _initialize_chemistry_data(chemistry_data *my_chemistry,
-                               chemistry_data_storage *my_rates,
-                               code_units *my_units)
+int local_initialize_chemistry_data(chemistry_data *my_chemistry,
+                                    chemistry_data_storage *my_rates,
+                                    code_units *my_units)
 {
 
   if (grackle_verbose) {
@@ -249,15 +249,17 @@ int _initialize_chemistry_data(chemistry_data *my_chemistry,
 
 int initialize_chemistry_data(code_units *my_units)
 {
-  if (_initialize_chemistry_data(grackle_data, &grackle_rates,
-                                 my_units) == FAIL) {
-    fprintf(stderr, "Error in _initialize_chemistry_data.\n");
+  if (local_initialize_chemistry_data(grackle_data, &grackle_rates,
+                                      my_units) == FAIL) {
+    fprintf(stderr, "Error in local_initialize_chemistry_data.\n");
     return FAIL;
   }
   return SUCCESS;
 }
 
 // Define helpers for the show_parameters function
+// NOTE: it's okay that these functions all begin with an underscore since they
+//       each have internal linkage (i.e. they are each declared static)
 static void _show_field_INT(FILE *fp, const char* field, int val)
 { fprintf(fp, "%-33s = %d\n", field, val); }
 static void _show_field_DOUBLE(FILE *fp, const char* field, double val)
@@ -273,9 +275,16 @@ void show_parameters(FILE *fp, chemistry_data *my_chemistry){
   #undef ENTRY
 }
 
+int free_chemistry_data(void){
+  if (local_free_chemistry_data(grackle_data, &grackle_rates) == FAIL) {
+    fprintf(stderr, "Error in local_free_chemistry_data.\n");
+    return FAIL;
+  }
+  return SUCCESS;
+}
 
-int _free_chemistry_data(chemistry_data *my_chemistry,
-			 chemistry_data_storage *my_rates) {
+int local_free_chemistry_data(chemistry_data *my_chemistry,
+                              chemistry_data_storage *my_rates) {
   if (my_chemistry->primordial_chemistry > 0) {
     GRACKLE_FREE(my_rates->ceHI);
     GRACKLE_FREE(my_rates->ceHeI);