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

added_new_bmi_output_variable #92

Merged
merged 4 commits into from
Jul 24, 2023
Merged

added_new_bmi_output_variable #92

merged 4 commits into from
Jul 24, 2023

Conversation

ajkhattak
Copy link
Contributor

The PR adds SOIL_TO_GW_FLUX (percolation) variable to BMI output list for our baseline simulations. The code was already computing this variable to so no changes to the functionality.

Additions

  • Added a new variable to the BMI output list

Removals

  • None

Testing

  1. The changes have been tested against the master branch, and the branch yields identical results

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)
  • Passes all existing automated tests
  • Reviewers requested with the Reviewers tool ➡️

@SnowHydrology
Copy link
Contributor

@ajkhattak Do we need to add any other output vars to CFE or is this it for NWM Baseline?

@ajkhattak
Copy link
Contributor Author

This is the only missing output variable in CFE for the NWM baseline

Copy link
Contributor

@SnowHydrology SnowHydrology left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ajkhattak, the changes look good, but the unit test build fails (see #89). Can you update that so we can unit test the updated BMI functionality? Thanks

@ajkhattak
Copy link
Contributor Author

@SnowHydrology sure, we haven't updated the unittest in a while, so let me see what's going on there.

@SnowHydrology SnowHydrology merged commit 338d93e into master Jul 24, 2023
4 checks passed
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.

2 participants