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

Add initial support for gage-assisted diversions in channel routing #756

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

rcabell
Copy link
Collaborator

@rcabell rcabell commented Jun 18, 2024

TYPE: ENHANCEMENT

KEYWORDS: diversions,DA,channel routing

SOURCE: NCAR

DESCRIPTION OF CHANGES:

Add initial support for channel diversions. This PR only covers the 'gage-assisted diversion' as used on the Mississippi at the Old River Control Structure.

TESTS CONDUCTED:

  • Croton with artificial diversion - MOSTLY COMPLETE
  • CONUS with actual Old River Control Structure diversion - IN PROGRESS

NOTES:

Diversions must be specified in a diversions_file added to the hydro.namelist. The format of this file will be documented later as part of the NWM v3.1 release.

This PR introduces a small amount of C code to find USGS Timeslice files, and as such, introduces a new model dependency on a C compiler and a POSIX-compliant operating system. This should not pose an undue burden on the vast majority of users.

scrasmussen
scrasmussen previously approved these changes Jun 20, 2024
Copy link
Member

@scrasmussen scrasmussen left a comment

Choose a reason for hiding this comment

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

Successfully built with nudging turned on and the comment I made is a minor thing if you think it is worth doing, LGTM!

src/Routing/Diversions/module_diversions.F Outdated Show resolved Hide resolved
CMakeLists.txt Outdated
@@ -205,13 +205,13 @@ if (CMAKE_Fortran_COMPILER_ID MATCHES "GNU.*")
if (CMAKE_Fortran_COMPILER_VERSION VERSION_GREATER_EQUAL 9)
set (CMAKE_Fortran_FLAGS "${CMAKE_Fortran_FLAGS} -fallow-argument-mismatch")
endif()
set (CMAKE_Fortran_FLAGS_RELEASE "-O2")
set (CMAKE_Fortran_FLAGS_RELEASE "-O2 -march=native")
Copy link
Member

Choose a reason for hiding this comment

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

Tested the removal of the -march=native flag and the tests passed. Seems the differences are just caused by that optimization

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great find! We may want to introduce that back eventually (after some benchmarking), as it theoretically can improve performance considerably on some systems.

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