Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 393 #394

Merged
merged 3 commits into from
Mar 11, 2022
Merged

Fix 393 #394

merged 3 commits into from
Mar 11, 2022

Conversation

hellkite500
Copy link
Member

@hellkite500 hellkite500 commented Mar 10, 2022

Fixes #393
Fixes #392

Changes

  • Rename get_bmi_var_name to get_bmi_output_var_name and make it a protected function
  • validate name map lookup resolves to an output variable
  • Removed the -0.0 allowance in C formulation unit test

Testing

  1. Tested with unit tests and local runs using example realizations, but with standalone CFE and CFE+PET multi-bmi

Notes

  • I had worked up a more generic get_bmi_alias function that validates names/alias by type (output, input, any). But I'm not sure if it is needed here or not...

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
  • Project documentation has been updated (including the "Unreleased" section of the CHANGELOG)
  • Reviewers requested with the Reviewers tool ➡️

Target Environment support

  • Linux
  • MacOS

@hellkite500
Copy link
Member Author

For reference, here is what I was thinking about for a generalized get_bmi_alias function for a Module Formulation

    protected:

        enum BMI_VAR_TYPE{ BMI_INPUT_VAR, BMI_OUTPUT_VAR, BMI_ANY_VAR};

        /**
         * @brief Get correct BMI variable name, which may be the output or something mapped to this output.
         * 
         * @param name 
         * @param bmi_var_name 
         */
        inline void get_bmi_alias(const std::string &name, std::string &bmi_var_name, BMI_VAR_TYPE type)
        {
            std::vector<std::string> names;
            switch(type){
                case BMI_OUTPUT_VAR:
                    //standard output names fi
                    names = get_bmi_model()->GetOutputVarNames();
                case BMI_INPUT_VAR:
                    names = get_bmi_model()->GetInputVarNames();
                case BMI_ANY_VAR:
                    names = get_bmi_model()->GetOutputVarNames();
                    std::vector<std::string> tmp = get_bmi_model()->GetInputVarNames();
                    //dump input names after output names
                    names.insert(names.end(), tmp.begin(), tmp.end());
            }
            if (std::find(names.begin(), names.end(), name) != names.end()) {
                bmi_var_name = name;
            }
            else
            {
                //check mapped/alias names
                std::string mapped_name;
                for (auto & iter : bmi_var_names_map) {
                    if (iter.second == name) {
                        mapped_name = iter.first;
                        break;
                    }
                }
                if (std::find(names.begin(), names.end(), mapped_name) != names.end()){
                    bmi_var_name = mapped_name;
                }
                //else not a BMI recognizable variable for given type
            }
        }

And then get_value would call it, knowing it is looking for valid output vars, like so

get_bmi_alias(output_name, bmi_var_name, BMI_OUTPUT_VAR);

@mattw-nws mattw-nws merged commit 05ec60f into NOAA-OWP:master Mar 11, 2022
@hellkite500 hellkite500 deleted the fix-393 branch March 15, 2022 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug in checking BMI output for forcing provider interface Failing unit tests on macos
2 participants