From 97257fcf84c818fbaf750baa97dd5903dc49c236 Mon Sep 17 00:00:00 2001 From: Matthew Abruzzo Date: Mon, 3 Jun 2024 09:02:29 -0400 Subject: [PATCH 1/2] Introduce gr_initialize_field_data This change was split off from GH PR #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 #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 #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) --- doc/source/Integration.rst | 3 ++ doc/source/Reference.rst | 19 +++++++ src/clib/grackle.h | 2 + src/clib/grackle_field_data_fdatamembers.def | 57 ++++++++++++++++++++ src/clib/grackle_fortran_interface.def | 8 +++ src/clib/set_default_chemistry_parameters.c | 22 ++++++++ src/example/c_example.c | 3 +- src/example/c_local_example.c | 1 + src/example/cxx_grid_example.C | 1 + src/example/cxx_omp_example.C | 2 + src/example/fortran_example.F | 1 + src/python/pygrackle/grackle_defs.pxd | 2 + src/python/pygrackle/grackle_wrapper.pyx | 1 + 13 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 src/clib/grackle_field_data_fdatamembers.def diff --git a/doc/source/Integration.rst b/doc/source/Integration.rst index 4e1f4567..5c4e03b1 100644 --- a/doc/source/Integration.rst +++ b/doc/source/Integration.rst @@ -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; diff --git a/doc/source/Reference.rst b/doc/source/Reference.rst index 8af0674b..d9b6b101 100644 --- a/doc/source/Reference.rst +++ b/doc/source/Reference.rst @@ -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 @@ -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) diff --git a/src/clib/grackle.h b/src/clib/grackle.h index 349e2e0a..b9608015 100644 --- a/src/clib/grackle.h +++ b/src/clib/grackle.h @@ -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 diff --git a/src/clib/grackle_field_data_fdatamembers.def b/src/clib/grackle_field_data_fdatamembers.def new file mode 100644 index 00000000..2bea5615 --- /dev/null +++ b/src/clib/grackle_field_data_fdatamembers.def @@ -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) \ No newline at end of file diff --git a/src/clib/grackle_fortran_interface.def b/src/clib/grackle_fortran_interface.def index 97195dd0..6629c69e 100644 --- a/src/clib/grackle_fortran_interface.def +++ b/src/clib/grackle_fortran_interface.def @@ -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 \ No newline at end of file diff --git a/src/clib/set_default_chemistry_parameters.c b/src/clib/set_default_chemistry_parameters.c index 4ee9f7a1..56523666 100644 --- a/src/clib/set_default_chemistry_parameters.c +++ b/src/clib/set_default_chemistry_parameters.c @@ -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; +} diff --git a/src/example/c_example.c b/src/example/c_example.c index 1109305b..4eb7540d 100644 --- a/src/example/c_example.c +++ b/src/example/c_example.c @@ -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. @@ -244,4 +245,4 @@ int main(int argc, char *argv[]) fprintf(stderr, "dust_temperature = %g K.\n", dust_temperature[0]); return EXIT_SUCCESS; -} \ No newline at end of file +} diff --git a/src/example/c_local_example.c b/src/example/c_local_example.c index 13149488..d29f763e 100644 --- a/src/example/c_local_example.c +++ b/src/example/c_local_example.c @@ -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. diff --git a/src/example/cxx_grid_example.C b/src/example/cxx_grid_example.C index fa0770d7..0a85a77b 100644 --- a/src/example/cxx_grid_example.C +++ b/src/example/cxx_grid_example.C @@ -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. diff --git a/src/example/cxx_omp_example.C b/src/example/cxx_omp_example.C index 91e30737..98c6fe53 100644 --- a/src/example/cxx_omp_example.C +++ b/src/example/cxx_omp_example.C @@ -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. diff --git a/src/example/fortran_example.F b/src/example/fortran_example.F index 5e600ac5..e075325b 100644 --- a/src/example/fortran_example.F +++ b/src/example/fortran_example.F @@ -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. diff --git a/src/python/pygrackle/grackle_defs.pxd b/src/python/pygrackle/grackle_defs.pxd index 1739e9b7..13bd4d5b 100644 --- a/src/python/pygrackle/grackle_defs.pxd +++ b/src/python/pygrackle/grackle_defs.pxd @@ -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) diff --git a/src/python/pygrackle/grackle_wrapper.pyx b/src/python/pygrackle/grackle_wrapper.pyx index bf33c55e..a12219b4 100644 --- a/src/python/pygrackle/grackle_wrapper.pyx +++ b/src/python/pygrackle/grackle_wrapper.pyx @@ -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 From 80daa3c3fcf43cd523ba337a3c22eb6e2337379c Mon Sep 17 00:00:00 2001 From: Matthew Abruzzo Date: Tue, 25 Jun 2024 10:48:39 -0400 Subject: [PATCH 2/2] removed unused information from grackle_field_data_fdatamembers.def --- src/clib/grackle_field_data_fdatamembers.def | 68 ++++++++++---------- src/clib/set_default_chemistry_parameters.c | 2 +- 2 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/clib/grackle_field_data_fdatamembers.def b/src/clib/grackle_field_data_fdatamembers.def index 2bea5615..5d34a818 100644 --- a/src/clib/grackle_field_data_fdatamembers.def +++ b/src/clib/grackle_field_data_fdatamembers.def @@ -5,11 +5,9 @@ / 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 -/ +/ Currently, we just specify each field's name. In the future, we will +/ probably add other metadata (that may be used to help track when the +/ field is required or help with unit scaling/applying floors) / / Copyright (c) 2013, Enzo/Grackle Development Team. / @@ -19,39 +17,39 @@ / 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(density) +ENTRY(HI_density) +ENTRY(HII_density) +ENTRY(HM_density) +ENTRY(HeI_density) +ENTRY(HeII_density) +ENTRY(HeIII_density) +ENTRY(H2I_density) +ENTRY(H2II_density) +ENTRY(DI_density) +ENTRY(DII_density) +ENTRY(HDI_density) +ENTRY(e_density) +ENTRY(metal_density) +ENTRY(dust_density) -ENTRY(internal_energy, 0) -ENTRY(x_velocity, 0) -ENTRY(y_velocity, 0) -ENTRY(z_velocity, 0) +ENTRY(internal_energy) +ENTRY(x_velocity) +ENTRY(y_velocity) +ENTRY(z_velocity) -ENTRY(volumetric_heating_rate, 0) -ENTRY(specific_heating_rate, 0) +ENTRY(volumetric_heating_rate) +ENTRY(specific_heating_rate) -ENTRY(temperature_floor, 0) +ENTRY(temperature_floor) -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(RT_heating_rate) +ENTRY(RT_HI_ionization_rate) +ENTRY(RT_HeI_ionization_rate) +ENTRY(RT_HeII_ionization_rate) +ENTRY(RT_H2_dissociation_rate) -ENTRY(H2_self_shielding_length, 0) -ENTRY(H2_custom_shielding_factor, 0) +ENTRY(H2_self_shielding_length) +ENTRY(H2_custom_shielding_factor) -ENTRY(isrf_habing, 0) \ No newline at end of file +ENTRY(isrf_habing) diff --git a/src/clib/set_default_chemistry_parameters.c b/src/clib/set_default_chemistry_parameters.c index 56523666..964338c0 100644 --- a/src/clib/set_default_chemistry_parameters.c +++ b/src/clib/set_default_chemistry_parameters.c @@ -57,7 +57,7 @@ int gr_initialize_field_data(grackle_field_data *my_fields) // 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; + #define ENTRY(MEMBER_NAME) my_fields->MEMBER_NAME = NULL; #include "grackle_field_data_fdatamembers.def" #undef ENTRY