Skip to content

A better rate function API for 4.0 #414

@mabruzzo

Description

@mabruzzo

Personally, I've never been a huge fan of the rate functions API. I'm not opposed to exposing this kind of functionality, but I think that the current approach of exposing every rate as a distinct function leaves a lot to be desired.

I know that this API was the simplest thing for us to do; the API let us expose internal logic to users. While I'm not fundamentally opposed to that kind of approach (after all, we aren't full-time software engineers), I think there are way too many of these functions to ignore the warts (after we merge in the newchem-cpp branch there will be 80+ collisional rates)

Concerns about the current API

My concerns about the current API fall into 2 categories:

  1. "Mechanics:"
    • I don't like that we are polluting the namespace of all c functions. Names like k13_rate or brem_rate are terribly generic, and its plausible we could produce name collisions.1
    • Ideally, we would probably be explicitly unit-testing (or performing basic smoke tests) on every public function.
    • We may need to stop passing chemistry_data as an argument (I think that the 4.0 release may replace the chemistry_data type)
  2. "Ergonomics:"
    • it's really easy to "shoot yourself in the foot." To know the proper units argument you need to explicitly read the documentation
    • the ergonomics of having 100+ functions is pretty terrible. If somebody wants to call every function, they need to write out 100 different function names in their codebase (and they need to figure out what the proper unit is for every single function)
    • I would argue that the tediousness of the current API is exemplified by the fact that we don't bother to explicitly list out every single function in our own API documentation

Thoughts on how to improve the API

I don't have all the full answer. But I might have a good way to start.

We could replace a lot of individual functions with one of the following

  1. a single function that does the calculation for a lot of different rates

    int gr_calc_collisional_rate(const char *rate_name, double* out, unsigned int length,
                                 const double* T, double kunit, chemistry_data* my_chemistry);

    where rate_name holds the name of the rate.2 The idea is that this call would store values in out.

  2. a function that returns a function pointer, represented by the rate_function type,3 that the end-user would invoke to compute rates.

    rate_function gr_get_rate_fn( const char *rate_name );

    Since this will be defined in a C++ file, we will probably need to declare each of the functions returned as function pointers as extern "C" (if we want this function to be usable in C libraries).4

Of course, we can add functions to do lots of other things (e.g. query the kind of units associated with a given rate, query reactants/products in a rate, ...). It all boils down to the intended use case for these functions, which has never been very clear to me.

If these functions are only intended for experimentation, it may make sense to explicitly make them part of an unstable API (so we have the freedom to change the functions between versions).

Footnotes

  1. If this were the ONLY problem, we could resolve it by simply changing the names to something like: gr_k13_rate gr_brem_rate.

  2. We could replace chemistry_data with the appropriate type if it is removed in 4.0.

  3. If performance is important, we probably want rate_function to compute values at several temperatures in a single call.

  4. Ideally, we wouldn't make any of names of the functions, that are returned as pointers, publicly visible to external code (i.e. the only way to call them is through a pointer returned by gr_get_rate_fn -- you can't simply specify the name). The most convenient way to do that is to declare them as static. Unfortunately you can't declare a function as both static and as extern "C". So we may want to replace static with clang/gcc attributes for controlling visibility (or we can live with the symbols being visible).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions