BMI: bug fixes and updates #306
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Both were missing the required `dest` argument.
Should return the datatype of a variable (with `eltype`) and not the type of a complete Array (`typeof`).
- fix functions that deviate from BasicModelInterface.jl (e.g. get_grid_x). - add metadata to Structs, for example grid location (node or edge), that can be used by BMI (and Wflow).
SVector type and 2D Array variables are exposed per third dimension coordinate, by adding "-i" (i refers to index) to the variable name (to minimize the number of required grid types). Extracting index i from the variable name has been fixed.
This is done for boundary/ghost nodes.
For SVector type and 2D Array variables use "[ì]" instead of "-i"
Replace getindex (values are retrieved from the array (copy)), with reinterpret (a copy is not created).
For model coupling (OpenDA) additonal functions for loading and saving state information and start time in Unix time format are required. For this, the set states functionality from the initialization function is moved to a separate `set_states` function for each Model type.
config.startime and config.endtime should be converted to `DateTime`, because Wflow accepts also Strings here (TOML).
2 tasks
visr
approved these changes
Nov 14, 2023
Co-authored-by: Martijn Visser <[email protected]>
Co-authored-by: Martijn Visser <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue addressed
Fixes #304, #307
Explanation
Issue #304:
Fixed Basic Model Interface (BMI) functions that deviated from BMI specifications (BasicModelInterface.jl). Model variables that represent 3-dimensional data (stored as 2d Arrays or Vector of
SVectors) are exposed as 2-dimensional unstructured grids. This minimizes the number of required grid types and users are most probably interested in data per third grid coordinate (for example per soil layer). BMI grid information (type and location) and whether a model variable can be exchanged is now stored as part of Struct metadata. For Wflow components that store model variables on gridedgesthe BMI model grid functions have been extended withget_grid_edge_countandget_grid_edge_nodes.Issue #307:
config.starttimeandconfig.endtimecan also be provided as Strings in the TOML file, so conversion toDateTimeis also required in BMI functions where these variables are used.Checklist
masterAdditional Notes
A total of 7 different grid types are required and from a user perspective fewer grid types would be better. To support fewer grid types, changes at the Wflow
Modellevel are required (see also #304 (comment)). This can be part of another PR.Also added state and time functions (extension of BMI) that can be used for coupling with OpenDA (and other external) software.