Pre-submission comments on BMI docs #58
Comments
|
I've numbered @gregtucker's comments, just to make them easier to reference as we work through the list. |
|
|
@mcflugen for (4) I was thinking situations where you have a formula like: y = b x^c where the units of b depend on the value of c. This comes up in empirical hydrology formulas, such as the oft-used expression for channel width (y) as a function of discharge (x). It's admittedly ugly to have coefficients with weird, exponent-dependent units, but seems to be common practice. |
This commit contains minor text updates; it doesn't include the reorg or updates to the CMF page suggested in #58; since those are bigger, I'll handle them separately.
I've gathered all these changes into a PR, #59. |
|
+1 |
A few comments and suggested edits on BMI docs:
"If the model’s state variables don’t change in time, then they can be computed by the initialize function and this function can just return without doing anything": Would an appropriate alternative design pattern, for situations where there is no time-stepping but a user might conceivably want to re-run a component (for example, after changing one or more variables using set_value) be to have update() re-run the calculations? If so, it's probably worth stating that after the above sentence in the docs.
get_input_item_count(): might be worth adding that this is the # of variables that can be set via set_value (if that's correct); similarly for get_value on get_output_item_count
"A model may have no input [or output] variables": could be mis-read as meaning "may not" equals "not allowed"; suggest "might" instead of "may", and/or adding something like "in which case the return value is empty"
get_var_units: is there an identifier for units that vary depending on context? If so, might be worth a mention (even just to note that this is also covered in UDUNITS, if it is)
get_var_location: the 3 possible returns don't include a scalar (i.e., single value), which makes me think that the variable information functions apply only to grid-based variables. So you could not, for example, use get_value to query the value of a single-valued parameter. I could imagine users getting confused about this, so the working definition of variable is probably worth explaining in the intro to the Variable Information Functions section.
"If the model doesn’t define an end time, a large number (e.g., 1.0e6) is typically chosen." - This makes sense, but I can imagine situations where this could cause issues---e.g., if a model's time units are seconds and it runs for more than a year (~pi x 10^7 sec). Would it make sense to suggest a default equal to the highest floating point number or something like that? Or maybe just an even bigger number: IEEE double-precision max exponent, according to wikipedia, is 308, so could suggest 1.0e308.
"Avoid using years as a unit, if possible, since a year is difficult to define precisely." - a lot of geologic-time models use years... Does UDUNITS define different forms of year, e.g., 365 days, 365.25 days, or a certain number of seconds for a proper astronomical year? If so, could suggest that here as an alternative for those models that insist on using years.
get_value: people might be confused by the reference to "buffer"; call it "the array parameter" instead?
"One grid could be a uniform rectilinear grid on which temperature is defined. A second grid could be a scalar, on which a constant thermal diffusivity is defined." - oh, that's cool! So maybe that's the answer to the question about scalar parameters: define a second grid that contains such-and-such variables. Not sure whether that's a practice we want to recommend, though...?
get_grid_type and following: "This function is needed for every grid type." - not sure what this means
get_grid_rank: is this the same as the # of dimensions in the grid arrays, ie 1d, 2d, or 3d? is it zero for a scalar grid?
get_grid_origin: what if you grid is 2d but its arrays are flat? Seems like it should be rank 1, but the origin should still have two values. I expect this would happen with unstructured grids, for example; might be worth a note somewhere about how this would normally be handled.
The reader has no way of knowing that the cool material under "Additional Topics" exists until they get to the bottom of the function description section. Suggest moving these links, or copying them, under the heading Basic Model Interface, between the end of the text here and Table 2. One way to organize would be a sub-head "BMI Functions" for the existing text, and then "Additional topics" with the links.
Grids: "The grid shape is the number of nodes..." - suggest "refers to the number of nodes", since the shape itself would be # rows and cols (eg). Or specify "number of rows and columns of nodes, as opposed to other types of element (such as cells or faces)"
Great illustrations in the grid section!
CSDMS Modeling Framework: great to refer to this, but I suspect if someone searched for what this is and where to try it out, they wouldn't find anything, because they don't know that CMF basically means "pymt" (these days). How about a sentence pointing them to pymt and its tutorials?
Also, suggest adding a "How to contact us for help incorporating a component" type of thing at the end of the CMF page.
The text was updated successfully, but these errors were encountered: