You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Our current implementations of set_value and set_value_at_indices are not consistent with the BMI 2.0 standard and should be adjusted for compliance.
Current behavior
The bmi_cfe.c code treats set_value as a special case of set_value_at_indices where the former calls the latter. This is not correct.
This non-standard set_value implementation also makes the sometimes-okay-but-not-always assumption that all variables are scalar by hard-coding the inds variable dimension and the length argument in the function call to set_value_at_indices.
Furthermore, in this configuration, CFE parameters are settable via set_value_at_indices, which only works as a result of the non-standard implementation of set_value. Also of note is that the index values are not passed to the variables in set_value_at_indices, which only works if the variable is a scalar.
Expected behavior
set_value should copy the values from a source array to a destination array in the model, corresponding to the given variable (obviously the array sizes must match).
set_value_at_indices, similarly, sets a value or values, but only corresponding to the index or indices provided.
In C, both functions rely on the memcpy function. Given the syntax (memcpy (dest, array, nbytes); for set_value vs. memcpy (CHAR_CAST to + offset, ptr, itemsize); for set_value_at_indices), it appears more that the latter is a specific case of the former, rather than vice versa.
The set_value function should follow the code provided in TOPMODEL and in the BMI C example.
The set_value_at_indices function should also follow the code provided in TOPMODEL and in the BMI C example.
These changes will additionally require an update to the implementation of calibratable parameters in CFE. At minimum, these should be moved to set_value from set_value_at_indices.
Our current implementations of
set_value
andset_value_at_indices
are not consistent with the BMI 2.0 standard and should be adjusted for compliance.Current behavior
The
bmi_cfe.c
code treatsset_value
as a special case ofset_value_at_indices
where the former calls the latter. This is not correct.This non-standard
set_value
implementation also makes the sometimes-okay-but-not-always assumption that all variables are scalar by hard-coding theinds
variable dimension and the length argument in the function call toset_value_at_indices
.Furthermore, in this configuration, CFE parameters are settable via
set_value_at_indices
, which only works as a result of the non-standard implementation ofset_value
. Also of note is that the index values are not passed to the variables inset_value_at_indices
, which only works if the variable is a scalar.Expected behavior
set_value
should copy the values from a source array to a destination array in the model, corresponding to the given variable (obviously the array sizes must match).set_value_at_indices
, similarly, sets a value or values, but only corresponding to the index or indices provided.In C, both functions rely on the
memcpy
function. Given the syntax (memcpy (dest, array, nbytes);
forset_value
vs.memcpy (CHAR_CAST to + offset, ptr, itemsize);
forset_value_at_indices
), it appears more that the latter is a specific case of the former, rather than vice versa.The
set_value
function should follow the code provided in TOPMODEL and in the BMI C example.The
set_value_at_indices
function should also follow the code provided in TOPMODEL and in the BMI C example.These changes will additionally require an update to the implementation of calibratable parameters in CFE. At minimum, these should be moved to
set_value
fromset_value_at_indices
.Further discussion can be found here.
The text was updated successfully, but these errors were encountered: