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

Diffusive fortran kernel correctly executes diffusive wave on stream segments defined by Python preprocessing #758

Merged
merged 3 commits into from
Apr 16, 2024

Conversation

kumdonoaa
Copy link
Contributor

When defining diffusive domain including not only mainstem but also connected tributaries by this config file, diffusive.f90 hasn't been executed diffusive wave routing on all the reaches where the config file defines to run diffusive wave on. That's because diffusive.f90 assumes a reach without any its upstream reach to not run diffusive wave on but instead to already have given flow and depth data computed from other routing model, in this case MC. By defining a reach of index j in frnw_g with specified value, for example, 555 for diffusive reach while -555 for already-computed reach, diffusive.f90 now correctly route on all reaches told to run diffusive wave.

Additions

AbstractRouting.py: Select a single cross section topo data with minimum value of cs_id for each stream segment
diffusive_utils_v02.py: Decide a stream reach of index j whether to run diffusive wave or not to run diffusive but have already-computed flow value by MC instead.
diffusive.90: Implement diffusive wave on all the reaches that are told to run diffusive on.

Removals

Changes

Testing

Screenshots

Notes

Todos

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
  • Browser

Accessibility

  • Keyboard friendly
  • Screen reader friendly

Other

  • Is useable without CSS
  • Is useable without JS
  • Flexible from small to large screens
  • No linting errors or warnings
  • JavaScript tests are passing

@JurgenZach-NOAA
Copy link
Contributor

When mainstem: 'mainstem' is included in the yaml file, it runs.

Copy link
Contributor

@JurgenZach-NOAA JurgenZach-NOAA left a comment

Choose a reason for hiding this comment

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

Code runs, recommend mainstem: 'mainstem' in all test examples now.

@kumdonoaa kumdonoaa merged commit 3b6021a into NOAA-OWP:master Apr 16, 2024
4 checks passed
@kumdonoaa kumdonoaa deleted the fix_nandiffusive_flow branch April 16, 2024 18:49
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.

4 participants