Bmi conventions doc #507
Bmi conventions doc #507
Conversation
mattw-nws
force-pushed
the
bmi-conventions-doc
branch
from
2c86f22 to
3f1173c
Compare
|
|
||
| Note that there is currently no discoverability mechanism for unpublished variables, they must be described in documentation (such as this document or a module's documentation) and often manually implemented or configured for use (such as in a realization config file). | ||
|
|
||
| ## Unknown/unexpected variables passed to `set_value` |
There was a problem hiding this comment.
Umm...I'm not sure I agree with this statement. Would be happy to discuss in more detail.
There was a problem hiding this comment.
The header? Or the section contents? Sure, here, or hit me up on Slack.
There was a problem hiding this comment.
Why should set_value not return BMI_FAILURE? If ngen is using unpublished conventional variables, these are essentially OPTIONAL and it should explicitly check to ensure that BMI_SUCCESS was returned from set_value to ensure that it can properly assume the semantics are valid. BMI_FAILURE would indicate that those assumptions are not valid. That is how calibration parameters are handled right now -- BMI_FAILURE just assumes that variable is not calibratable. When we get to grid conventions, it is almost imperative for BMI_FAILURE to be returned if a call to set_value( 'grid_X_shape') fails...this allows the model engine to know that the grid cannot be manipulated by the framework, and it can test if the model implements the correct grid functions and their values align in a meaningful fashion.
There was a problem hiding this comment.
Ah. Okay, this part I was less sure about. Generally, I wasn't sure how ngen would handle set_value returning BMI_FAILURE and whether it would consider that an error for a convention variable. You are probably right here, especially if the ngen code already expects these semantics.
The only thing left in my mind is, if the module supports the variable, but encounters an unexpected problem, what then? For C++/Python we can recommend specific exceptions but we're kindof out of options for C/Fortran. Probably no good answer, but it's a likely scenario. Will it be important to distinguish between support+failure vs. nosupport?
There was a problem hiding this comment.
Nope, not without enumerated error codes...BMI 3.0 recommendation =)
doc/BMIconventions.md
Outdated
|
|
||
| > BMI functions always use flattened, one-dimensional arrays...It’s the developer’s responsibility to ensure that array information is flattened/redimensionalized in the correct order. | ||
|
|
||
| However, strictly speaking, *how* a multi-dimensional array should be flattened into a one-dimensional one is never directly addressed. **For ngen, it is required that flattening happens as if the array was a C array, sometimes referred to as "row major order".** That is, if you create a multi-dimensional array in C (or a C array in C++) it will already be in the appropriate order and layout such that if the data is copied into a 1D array (or the pointer is passed as the result of `get_value_ptr`) it is already in the correct order/layout and will "just work". |
There was a problem hiding this comment.
It is not guaranteed to be contiguous though if you are using pointers. Should probably note that caveat.
There was a problem hiding this comment.
Ah, had to reread... okay, yes, if you create a C array of pointers that's different. Is there a name for the "correct" allocation method?
doc/BMIconventions.md
Outdated
|
|
||
| BMI modules written in C, Fortran, and C++ *must* be compiled with the exact same header files as those used with ngen itself, and this means the BMI v2.0 header files provided in the CSDMS [`bmi-c`](https://github.com/csdms/bmi-c/blob/e6f9f8a0ab326218831000b4a5571490ebc21ea2/bmi.h), [`bmi-fortran`](https://github.com/csdms/bmi-fortran/blob/e34cd026f57cabd009ef0f662fc672baee66e442/bmi.f90), and [`bmi-cxx`](https://github.com/csdms/bmi-cxx/blob/be631dc510b477b3bc3eb3c8bbecf3d04ec4005c/bmi.hxx) repositories. Python models must inherit from the `Bmi` class in the [`bmipy`](https://pypi.org/project/bmipy/2.0/) package. Python and C++ classes can be subclasses of the standard BMI interface, but such subclasses must inherit from the standard versions listed here. | ||
|
|
||
| This means among other things that it is not possible to "extend" the BMI interface simply by adding functions to the base interface or header files. In any case, even with subclassed BMI models in Python and C++ with additional functions, ngen would not call any such additional functions because it does not know that they exist. |
There was a problem hiding this comment.
"This means, among other things, that it is..." A little comma grammar...
There was a problem hiding this comment.
My problem is usually more overuse of commas. This looks like a job for... EMDASH! 🦸♂️
Content suggestions from review. Co-authored-by: Phil Miller - NOAA <unmobile+gh@gmail.com>
[Short description explaining the high-level reason for the pull request]
Additions
Removals
Changes
Testing
Screenshots
Notes
Todos
Checklist
Testing checklist (automated report can be put here)
Target Environment support