Update BMI specification to include calibratable parameters

[SnowHydrology]

This modification allows the Nextgen model engine to change parameter values by exposing them through BMI.

Additions

All in bmi_topmodel.c:

• PARAM_VAR_NAME_COUNT
• param_var_names
• param_var_types

Changes

Updated the following in bmi_topmodel.c:

• Get_var_type
• Get_value_ptr

Testing

1. IN PROGRESS

Todos

• Add testing functionality to ensure param values can be get/set

Checklist

• PR has an informative and human-readable title
• Changes are limited to a single goal (no scope creep)
• Code can be automatically merged (no conflicts)
• Code follows project standards (link if applicable)
• Passes all existing automated tests
• Any change in functionality is tested
• New functions are documented (with a description, list of inputs, and expected output)
• Placeholder code is flagged / future todos are captured in comments
• Visually tested in supported browsers and devices (see checklist below 👇)
• Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
• Reviewers requested with the Reviewers tool ➡️

Testing checklist

Target Environment support

• Windows
• Linux
• MacOS

Closes #28

[SnowHydrology]

@hellkite500, how did you test the functionality in NOAA-OWP/cfe#44?

[hellkite500]

@SnowHydrology I ran it through the ngen driver and the calibration scripts...though if you have a test setup, you can always call Set_value on the param names and then try Get_value to see if you get back the same thing you set...

[SnowHydrology]

@madMatchstick made a really nice, reproducible unit testing script for the BMI implementation in C models. To get the number of input and output vars and their names, for example, it calls get_*put_item_count and get_*put_var_name. The script can then loop through the arrays it creates to confirm other functions are working. Is there any reason we wouldn't want to add get_param_item_count and get_param_var_name to the BMI specification?

[hellkite500]

If you are implying adding them to bmi.h such as these functions, then yes, the reason not to add them there is because then this module would not be ABI compatible with the ngen bmi 2.0 adapters.

[SnowHydrology]

What if I were to put them in include/bmi_topmodel.h like init_config and read_init_config?

https://github.com/NOAA-OWP/topmodel/blob/master/include/bmi_topmodel.h

I'm just trying to think of ways to test the functions on the model side and on the ngen side.

[hellkite500]

There are several ways to do it...you can add helper functions to bmi_topmodel like this

bmi_topmodel.h

char* get_param_names();
int get_param_count();

bmi_topmodel.c

int get_param_cout(){
    return PARAM_VAR_NAME_COUNT;
}

char* get_param_names()
{
    return param_var_names;
}

then in test/main_unit_test_bmi.c, around line 32, you can implement some param testing

    char* params = get_param_names();
    int num_params = get_param_count();
    int expected_num_params = 5;
    static const char *expected_param_names[expected_num_params] = {"szm", "td", "srmax", "sr0", "xk0"};
    assert(num_params == expected_num_params);
    double test_set_value = 4.2;
    double test_get_value = 0.0;
    for( int i = 0; i < num_params; i++ ){
        assert(strcmp(params[i], expected_param_names) == 0);
        status = model->set_value(model, params[i], &test_set_value);
        assert(status == BMI_SUCCESS);
        status = model->get_value(model, params[i], & test_get_value);
        assert(status == BMI_SUCCESS);
        assert(test_set_value == test_get_value);
    }

You can also skip the helper functions and just define the assumptions in the test...

    int expected_num_params = 5;
    static const char *expected_param_names[expected_num_params] = {"szm", "td", "srmax", "sr0", "xk0"};
    double test_set_value = 4.2;
    double test_get_value = 0.0;
    for( int i = 0; i < expected_num_params; i++ ){
        status = model->set_value(model, params[i], &test_set_value);
        assert(status == BMI_SUCCESS);
        status = model->get_value(model, params[i], & test_get_value);
        assert(status == BMI_SUCCESS);
        assert(test_set_value == test_get_value);
    }

This should still fail if the model isn't able to set values on the required params...

[SnowHydrology]

@hellkite500, thanks for that! I'll work on putting together a test with helper functions that doesn't touch include/bmi.h.

[madMatchstick]

added get/set values for new params in bmi unit testing sh as outlined in //TODO

[SnowHydrology]

Thanks @madMatchstick!