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

Bug fix for bypass_routing_option = 'direct_to_outlet' method #57

Merged
merged 8 commits into from
Nov 12, 2022

Conversation

olyson
Copy link
Contributor

@olyson olyson commented Aug 24, 2022

This fixes the balance errors evident in the mosart log file when the bypass_routing_option='direct_to_outlet' method is used.
Note that these balance errors are warnings and do not stop the model.
Code from @swensosc
PR requested by @ekluzek

Fixes #54
Fixes #56

Test case:
/glade/work/oleson/ctsm5.1.dev100/cime/scripts/ctsm51_ctsm51d100_1deg_GSWP3V1_directtooutlet_2000
Note that the test case uses ctsm5.1.dev100 and mosart1_0_45.
Verified that there were no balance errors in the mosart log file.
Testing: Standard mosart test list on cheyenne PASS with expected change in answers for
ERP_D.f10_f10_mg37.I1850Clm50Bgc.cheyenne_intel.mosart-qgrwlOpts

Also don't allow the threshold option to be set with
direct_to_outlet is choosen, as well as any option other
than "all" for "none".

Answers change when direct_to_outlet option is used

@ekluzek
Copy link
Contributor

ekluzek commented Oct 5, 2022

@olyson does this have everything needed in place? Naoki tried using this branch and was still seeing trouble with direct_to_outlet method for MOSART.

@olyson
Copy link
Contributor Author

olyson commented Oct 5, 2022

My understanding from Sean is that these two things need to be set in the mosart namelist:

bypass_routing_option = 'direct_to_outlet'
qgwl_runoff_option = 'negative'

@ekluzek
Copy link
Contributor

ekluzek commented Oct 6, 2022

Note, that Naoki ran into trouble when he ran with qgwl_runoff_option="threshold" and direct_to_outlet, but it works fine when run with negative. @swensosc is there enough of a reason to get the "threshold" option to work with direct_to_outlet? If we leave it as a problem we should document that it's a problem.

@swensosc
Copy link
Contributor

swensosc commented Oct 7, 2022

The 'negative' option bypasses the routing model, and sends all negative runoff either directly to the coupler, or to the river mouth (which then goes to the coupler). The 'threshold' option does the additional step of first checking the amount of water in the routing model locally. If there is available water (above a specified threshold, hence the name), that water is used to offset some of the negative runoff. This acts to reduce the amount of negative runoff that is passed to the coupler. That is effectively what is happening in the 'direct_to_outlet' method anyway, so it seems like it would be unnecessary for the 'direct_to_outlet' option.

@ekluzek
Copy link
Contributor

ekluzek commented Oct 7, 2022

@swensosc should we then remove the "threshold" option for use with direct_to_outlet? Especially, since there are problems when you use it, we might as well disallow its use at least for direct_to_outlet. I'm assuming it's still useful for direct_in_place.

@swensosc
Copy link
Contributor

swensosc commented Oct 7, 2022 via email

@ekluzek
Copy link
Contributor

ekluzek commented Oct 7, 2022

Perfect that's what I needed to know. I'll disable setting it for anything but direct_i_place.

@ekluzek ekluzek marked this pull request as ready for review November 11, 2022 23:12
@ekluzek
Copy link
Contributor

ekluzek commented Nov 12, 2022

Note this changes answers for the test case: ERP_D.f10_f10_mg37.I1850Clm50Bgc.cheyenne_intel.mosart-qgrwlOpts

 RMS rofImp_Forr_rofi                 3.8131E-06            NORMALIZED  3.2797E+01
 RMS rofImp_Forr_rofl                 8.2276E-06            NORMALIZED  3.6195E+00

My guess is that everything else will be identical

@ekluzek
Copy link
Contributor

ekluzek commented Nov 12, 2022

The ERS_Ld5.f10_f10_mg37.I2000Clm50BgcCru.cheyenne_intel.mosart-iceOff test is exact and I expect all other tests will be as well.

@ekluzek ekluzek merged commit 5b81cc5 into ESCOMP:master Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Some issues with direct_to_outlet option Update cime directory in buildlib and buildnml
3 participants