Renaming `_set_default_chemistry_parameters` and `_initialize_chemistry_data`

[mabruzzo]

Technically, the C standard reserves all externally visible function names that begin with an underscore (see the Reserved identifiers subsection on this webpage).

In other words _set_default_chemistry_parameters and _initialize_chemistry_data technically produce undefined behavior. In practice, this isn't much of an issue. But, it's probably worth addressing since these functions need to be used in conjunction with the local API.

Suggested Change:

I would suggest changing _initialize_chemistry_data to be local_initialize_chemistry_data.

I'd suggest that we tweak the name and signature of _set_default_chemistry_parameters to be:

int local_set_default_chemistry_parameters(chemistry_data *my_chemistry);

Alternatively, we could keep the signature and rename it to get_default_chemistry_parameters.

Maintaining Backwards Compatibility

I would suggest that we add something like the following to the header file:

#ifndef OMIT_LEGACY_INTERNAL_GRACKLE_FUNC

inline __attribute__((deprecated)) chemistry_data
_set_default_chemistry_parameters(void) {
  chemistry_data my_chemistry;
  local_set_default_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); }

#endif

the presence of the OMIT_LEGACY_INTERNAL_GRACKLE_FUNC would let the client code opt out of defining these functions

[brittonsmith]

Thanks for spotting this. I agree with your suggested strategy of keeping the old functions defined within an ifdef. I like the use of the deprecated variable attribute. I would also be in favor of a compile-time warning being issued when OMIT_LEGACY_INTERNAL_GRACKLE_FUNC is not defined, specifically warning users when these functions will be removed.

If we're changing the signature of _set_default_chemistry_parameters to what you suggest, I'd recommend something like local_initialize_default_chemistry_parameters or even just local_initialize_chemistry_parameters to match local_initialize_chemistry_data, which also accepts a pointer to chemistry_data.

I'm ok with this going ahead prior to the upcoming release with a warning that deprecated functions will be removed in the subsequent minor release.

[brittonsmith]

Resolved in PR #139.