Skip to content

Commit

Permalink
Introduce gr_initialize_field_data
Browse files Browse the repository at this point in the history
This change was split off from GH PR grackle-project#197. This should be a lot easier
to review in isolation (and I think that it is a generally useful
change in its own right).

Overview
--------
The objective of this PR is conceptually very simple. It introduces the
following function:

```c++
int gr_initialize_field_data(grackle_field_data *my_fields);
```

The idea is to have users immediately call this function right after they
initialize a new `grackle_field_data` instance

- When it's called the handful of non data-field members are assigned
  sensible defaults. It also initializes all data-field members to NULL
  pointers.

- this is analogous to the way that `local_initialize_chemistry_parameters`
  is used.

Motivation
----------

While this function is not strictly necessary, I think it is a useful
addition and I think we should tell people to use it (though old code
won't break).

A large motivation is peace of mind. I have often wished for this sort
of thing. I'm always a little concerned when using a new configuration
of grackle that I didn't initialize all of the required fields. I
would definitely feel a little more comfortable knowing that a field
that I forgot about is assigned a ``NULL`` pointer rather than some
garbage data that might let the code chug along.

It could also facillitate more error checking:

- such as checks in the style of the ``grid_dx`` check introduced in
  PR grackle-project#190 (applying similar checks to `grid_rank`, `grid_dimension`,
  `grid_start`, and `grid_end` requires that we know their default
  value).

- we could explicitly check that the user specified all required
  data-fields. We can only do this if we know that unset data-fields
  have a default value of `NULL`.

  - **From a user friendliness perspective, I think this check alone is
    enough to justify the function's existence.**

  - We could also potentially warn if unnecessary fields were specified

  - Plus, we could account for the fact that functions like
    ``calculate_pressure`` require fewer fields that ``solve_chemistry``

- we could also add checks that none of the fields are aliases (an
  implicit Fortran requirement). This would be much easier to implement
  if we knew we could just ignore fields that were set to ``NULL``
  pointers

We would probably need to make these checks opt-in somehow, both because
we can't be absolutely certain whether a user actually invoked
``gr_initialize_field_data`` and users might not want the runtime-cost
(which could be relatively large for very small grids)

Naming
------

I choose to name this function based on my suggestion in grackle-project#189 that we
adopt a convention that all new functions in the stable public API share
a common prefix (`gr_` or `grackle_`).

For the sake of proposing something concrete, I assumed that we choose
the `gr_` prefix. (If we decide on a different prefix, this would be
easy to change).

Alternatively, if we don't want any prefix, we could name it
`initialize_grackle_field_data` (I do worry a little that could
introduce naming conflicts with existing functions in downstream codes)
  • Loading branch information
mabruzzo committed Jun 3, 2024
1 parent 94c2ac4 commit 97257fc
Show file tree
Hide file tree
Showing 13 changed files with 121 additions and 1 deletion.
3 changes: 3 additions & 0 deletions doc/source/Integration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,9 @@ not intend to use.
// Create struct for storing grackle field data
grackle_field_data my_fields;

// initialize members of my_fields to sensible defaults
gr_initialize_field_data(&my_fields);

// Set grid dimension and size.
// grid_start and grid_end are used to ignore ghost zones.
int field_size = 1;
Expand Down
19 changes: 19 additions & 0 deletions doc/source/Reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ Grackle has two versions of most functions.
and :c:data:`chemistry_data_storage` instances to be provided as
arguments. These are explicity thread-safe as they use no global data.

Additionally, there are also some :ref:`misc_functions` and :ref:`rate-functions`.

.. _primary_functions:

Primary Functions
Expand Down Expand Up @@ -417,3 +419,20 @@ The following functions are used to query the name of the ith field of the :c:da
:param unsigned int i: The index of the accessed parameter
:rtype: const char*
:returns: Pointer to the string-literal specifying the name. This is ``NULL``, if :c:data:`chemistry_data` has ``i`` or fewer ``string`` members.

.. _misc_functions:

Miscellaneous Functions
-----------------------

.. c:function:: int gr_initialize_field_data(grackle_field_data *my_fields);
Initializes the struct-members stored in the :c:type:`grackle_field_data` data structure to their default values.
This function must assume that any existing data within the data structure is garbage data.
In other words, when this function goes to overwrite a given member of :c:type:`grackle_field_data`, it completely ignores the value currently held by the member (i.e. the function does not provide special handling for members holding non- ``NULL`` pointers).
Consequently, this function should **ONLY** be called **BEFORE** initializing any members of the data structure (if it's called at any other time, the program may leak memory resources).
:param grackle_field_data \*my_fields: uninitialized field data storage
:rtype: int
:returns: 1 (success) or 0 (failure)
2 changes: 2 additions & 0 deletions src/clib/grackle.h
Original file line number Diff line number Diff line change
Expand Up @@ -117,4 +117,6 @@ int local_free_chemistry_data(chemistry_data *my_chemistry, chemistry_data_stora

grackle_version get_grackle_version(void);

int gr_initialize_field_data(grackle_field_data *my_fields);

#endif
57 changes: 57 additions & 0 deletions src/clib/grackle_field_data_fdatamembers.def
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/***********************************************************************
/
/ this file lists each member of the grackle_field_data struct that is
/ intended to hold field data. This list is intended to be used with
/ X-Macros in *.c files (to reduce the amount of code required to
/ interact with these fields)
/
/ The following information is specified for each field:
/ 1. the field_name
/ 2. a 1 or 0 to denote whether the field_name corresponds to a
/ passive-scalar density field
/
/
/ Copyright (c) 2013, Enzo/Grackle Development Team.
/
/ Distributed under the terms of the Enzo Public Licence.
/
/ The full license is in the file LICENSE, distributed with this
/ software.
************************************************************************/

ENTRY(density, 0)
ENTRY(HI_density, 1)
ENTRY(HII_density, 1)
ENTRY(HM_density, 1)
ENTRY(HeI_density, 1)
ENTRY(HeII_density, 1)
ENTRY(HeIII_density, 1)
ENTRY(H2I_density, 1)
ENTRY(H2II_density, 1)
ENTRY(DI_density, 1)
ENTRY(DII_density, 1)
ENTRY(HDI_density, 1)
ENTRY(e_density, 1)
ENTRY(metal_density, 1)
ENTRY(dust_density, 1)

ENTRY(internal_energy, 0)
ENTRY(x_velocity, 0)
ENTRY(y_velocity, 0)
ENTRY(z_velocity, 0)

ENTRY(volumetric_heating_rate, 0)
ENTRY(specific_heating_rate, 0)

ENTRY(temperature_floor, 0)

ENTRY(RT_heating_rate, 0)
ENTRY(RT_HI_ionization_rate, 0)
ENTRY(RT_HeI_ionization_rate, 0)
ENTRY(RT_HeII_ionization_rate, 0)
ENTRY(RT_H2_dissociation_rate, 0)

ENTRY(H2_self_shielding_length, 0)
ENTRY(H2_custom_shielding_factor, 0)

ENTRY(isrf_habing, 0)
8 changes: 8 additions & 0 deletions src/clib/grackle_fortran_interface.def
Original file line number Diff line number Diff line change
Expand Up @@ -245,3 +245,11 @@ c The following define the fortran interfaces to the C routines
REAL(C_DOUBLE), INTENT(OUT) :: dust_temperature(*)
END FUNCTION calculate_dust_temperature
END INTERFACE

INTERFACE
INTEGER(C_INT) FUNCTION gr_initialize_field_data(my_fields)
& bind(C)
IMPORT
TYPE(grackle_field_data), INTENT(INOUT) :: my_fields
END FUNCTION gr_initialize_field_data
END INTERFACE
22 changes: 22 additions & 0 deletions src/clib/set_default_chemistry_parameters.c
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,25 @@ int set_default_chemistry_parameters(chemistry_data *my_grackle)
grackle_data = my_grackle;
return local_initialize_chemistry_parameters(my_grackle);
}

int gr_initialize_field_data(grackle_field_data *my_fields)
{
if (my_fields == NULL) {
fprintf(stderr, "gr_initial_field_data was passed a NULL pointer\n");
return FAIL;
}

my_fields->grid_rank = -1;
my_fields->grid_dimension = NULL;
my_fields->grid_start = NULL;
my_fields->grid_end = NULL;
my_fields->grid_dx = -1.0;

// now, modify all members holding datafields to have values of NULL
// (we use X-Macros to do this)
#define ENTRY(MEMBER_NAME, _1) my_fields->MEMBER_NAME = NULL;
#include "grackle_field_data_fdatamembers.def"
#undef ENTRY

return SUCCESS;
}
3 changes: 2 additions & 1 deletion src/example/c_example.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ int main(int argc, char *argv[])

// Create struct for storing grackle field data
grackle_field_data my_fields;
gr_initialize_field_data(&my_fields);

// Set grid dimension and size.
// grid_start and grid_end are used to ignore ghost zones.
Expand Down Expand Up @@ -244,4 +245,4 @@ int main(int argc, char *argv[])
fprintf(stderr, "dust_temperature = %g K.\n", dust_temperature[0]);

return EXIT_SUCCESS;
}
}
1 change: 1 addition & 0 deletions src/example/c_local_example.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ int main(int argc, char *argv[])

// Create struct for storing grackle field data
grackle_field_data my_fields;
gr_initialize_field_data(&my_fields);

// Set grid dimension and size.
// grid_start and grid_end are used to ignore ghost zones.
Expand Down
1 change: 1 addition & 0 deletions src/example/cxx_grid_example.C
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ grackle_field_data construct_field_data(grid_props& my_grid_props,

// Create struct for storing grackle field data
grackle_field_data my_fields;
gr_initialize_field_data(&my_fields);

// Set grid dimension and size.
// grid_start and grid_end are used to ignore ghost zones.
Expand Down
2 changes: 2 additions & 0 deletions src/example/cxx_omp_example.C
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,8 @@ int main(int argc, char *argv[])
}

grackle_field_data my_fields_t1, my_fields_tN;
gr_initialize_field_data(&my_fields_t1);
gr_initialize_field_data(&my_fields_tN);

// Set grid dimension and size.
// grid_start and grid_end are used to ignore ghost zones.
Expand Down
1 change: 1 addition & 0 deletions src/example/fortran_example.F
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ program fortran_example
iresult = initialize_chemistry_data(my_units)

c Set field arrays
iresult = gr_initialize_field_data(my_fields)

c If grid rank is less than 3, set the other dimensions,
c start indices, and end indices to 0.
Expand Down
2 changes: 2 additions & 0 deletions src/python/pygrackle/grackle_defs.pxd
Original file line number Diff line number Diff line change
Expand Up @@ -249,3 +249,5 @@ cdef extern from "grackle.h":
c_chemistry_data_storage *my_rates)

c_grackle_version c_get_grackle_version "get_grackle_version"()

int gr_initialize_field_data(c_field_data *my_fields)
1 change: 1 addition & 0 deletions src/python/pygrackle/grackle_wrapper.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -696,6 +696,7 @@ cdef c_field_data setup_field_data(object fc, int[::1] buf,

# now initialize my_fields
cdef c_field_data my_fields
gr_initialize_field_data(&my_fields)
my_fields.grid_rank = 1
my_fields.grid_dimension = grid_dimension
my_fields.grid_start = grid_start
Expand Down

0 comments on commit 97257fc

Please sign in to comment.