-
Notifications
You must be signed in to change notification settings - Fork 53
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
Including floodplain #470
base: cesm-coupling
Are you sure you want to change the base?
Including floodplain #470
Conversation
… in dfw_route.f90 was included with the previous commt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did a quick scan with @nmizukami and had a couple suggestions. But, looks good as far as I can see from a quick scan from an SE angle.
Naoki is going to make sure this is working OK and bring in as is. Later development for the science and tuning of it could happen in the future. But, getting the base version in would allow it to be used.
! replace depth with large values e.g., 10,000 [m] if floodplain mode is off | ||
if (.not.floodplain) then | ||
do iSeg=1,nSeg | ||
structSEG(iSeg)%var(ixSEG%depth)%dat(1) = 10000._dp ! [meter] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest making 10000._dp it a parameter called something like large_number just to denote that the value doesn't matter. Maybe use huge?
@@ -12,8 +12,6 @@ MODULE public_var | |||
|
|||
save | |||
|
|||
! ---------- common constants --------------------------------------------------------------------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this removed?
@@ -45,12 +45,6 @@ | |||
<varname_downSegId> Tosegment ! name of variable holding the ID of the next downstream segment | |||
<varname_pfafCode> PFAF ! name of variable holding the pfafstetter code | |||
! **************************************************************************************************************************** | |||
! Define options to include/skip calculations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove settings that are just for the standard defaults.
! ********************************************************************* | ||
! subroutine: check if variable exists | ||
! ********************************************************************* | ||
function check_variable(ncid, vname) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like having a subroutine for this kind of functionality.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be useful to add calls to this in other places as well, so that you could check if variables exist.
This PR includes:
This is similar approach to CaMAFlood, or HyMAT type floodplain mechanism. This does not intend to do HEC-RAS type channel geometry definition (with many channel cross sections information) that kills run for high resolution river network like MERIT hydro over global, due to memory and much high intense computation.
floodplain is not implemented to IRF (no concept of channel geometry) and KWT (seemingly more complex) at least for now
Address #396