csdms/bmi — pull request #1 by SiggyF (2014-07-27 14:53:54+00:00)
Title: BMI merge

I merged some of the changes that we made to use the BMI interface for our models. There are quite a few topics that can be implemented in different ways. Feel free to refuse this pull request and use it as a discussion topic.
I tested with the testcases and on my macbook, using gcc and clang everything compiles without warnings and 5 tests succeed.

The location of the bmi.h
--------------------------------
I moved the bmi.h to an include location. I think that it would be
elegant to use this header file as the formal specification of a BMI
compatible model. By using the c header file as the formal
specification, all languages can both implement and hook up BMI
compatible models. See the BMI examples in the
[OpenEarth repository](https://github.com/openearth/bmi/tree/master/models)
for implementation examples.

Global or instance
-----------------------
I removed the self from the C interface. This is in line with the
[documentation](http://csdms.colorado.edu/wiki/BMI_Description).
Having a model state, like other frameworks (ESMF, OpenMI) is from a
programming perspective more elegant and removing it made me feel
somewhat uncomfortable. Here are some arguments, why the model state
should not be used in the BMI interface:

- Most model implementations have global state, thus having an extra state argument is often useless.
- The update, finalize, returning int/void implies a global state or at least a current.
- The concept of an instance can be implemented in other ways, for
example using the approach used by matlab for the figure. I now use
`set_current_state`, `get_current_state`, which is useful for data
assimilation and rewinding interactive models.

Removing self also helps people move towards the BMI that have not
(yet) grasped the concept of Object Oriented programming.

Prefix
------
I removed the `BMI_` prefix on the functions (and made them all
lowercase). I believe that for some implementations the proper
approach would be to define an engine (component) specific prefix.
Having a single prefix is a problem when you are linking together a
number of components. Suppose I have component A and B that I want to
link together and they both have the function `BMI_Initialize`, that
will give a linking error. An easy solution is to define the functions
as `A_initialize` and `B_initialize`.

Export
------
I added some code so that the functions are exported, which is normal under gcc but for
MSVC you have to add some extra code. I put these and the calling convention in the BMI_API macro.

CMake
-----
I added a top level cmake file. For now this only descends into the `c` directory.

BMI, CMI, BMI+
--------------
I moved some of the content of the BMI header file into `bmi.h`, `cmi.h`, and `bmi_grid.h`.
The grid was not something that I planned to implement for some models. I think it makes sense to
have some parts as extensions. For example we use an extra set_logger method in all our BMI models
and some extra methods to allow for custom types. I imagine that we define these and the grid methods
as bmi+ extensions.

Roles and char**
----------------
I changed the `get_input_variable_names` into `get_var_name`,
`get_var_count` and `get_var_role`. The change from `get_var_names` to
`get_var_name` makes interoperability easier. Having `char**` does not
map elegantly to fortran for example. The change from `get_input_` to
`get_var_role` make implementation a bit easier. Most variables can be
used as in and output and it avoids having to loop over 2 lists.

Enum versus char*
-----------------
I changed a few return types from enums to chars.
For the variable types I propose we use a subset of the names used in [numpy](http://docs.scipy.org/doc/numpy/user/basics.types.html#array-types-and-conversions-between-types).
The advantage of using these type names is that they both contain size and type.
The roles are the same as used in fortran (in, out, inout).

void* versus double*
--------------------
I removed the get and set by value in favor of void* only. This reduces the number of functions that have to be implemented.
It also makes it clear where allocation should happen: get -> model allocates and deallocates, set -> caller allocates and deallocates.

update
------
I set the update to have a double time argument. This is I believe in line with the docs, with -1 as default timestep (which should always compare, even as a double).