-
Notifications
You must be signed in to change notification settings - Fork 313
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
Include PRISM precipitation data stream #1954
Conversation
I talking with @TeaganKing we want to add a system test for NIWO for NEON-Prism to our standard test list. |
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.
Looks good!
Thanks for reviewing this @wwieder ! |
It looks like the tests are currently failing due to an inability to connect to the repo 'https://storage.neonscience.org/neon-ncar/NEON/' . It sounds like this is either a proxy or network issue. Has this come up in previous NEON testing? Running a non-PRISM ABBY NEON test also failed on the same issue-- see /glade/scratch/tking/SMS_D_Mmpi-serial.CLM_USRDAT.I1PtClm51Bgc.cheyenne_intel.clm-default--clm/NEON/ABBY.20230410_120450_5k8j7k . This may be related to warnings such as "No .input_data_list files found in dir 'Buildconf'". |
…t casename is not being used (or if it's PRISM) for the user make sure the finidat file is appropriately spunup
@TeaganKing and @wwieder I pushed some changes to the branch that represent the things we talked about. Please look them over and let me know if you agree. Especially look at the warning that has to do with initial conditions. We can adjust it as we want. |
Hi @ekluzek , these changes look great to me! I just fixed a few typos. |
Rework handling of evaporation constraint in SoilFluxes Occasionally, h2osoi_ice was going significantly negative in UpdateState_TopLayerFluxes - see ESCOMP#1979. As noted in that issue, this seems to be due to h2osoi_ice having a very different magnitude from h2osoi_liq, leading to greater-than-roundoff-level differences from zero final state in a relative sense (i.e., relative to the magnitude of h2osoi_ice) - I think because of the appearance of the sum (h2osoi_ice + h2osoi_liq) in the equations that limit fluxes. To try to deal with this, I have reworked the handling of the evaporation constraint to directly limit both the liqevap and solidevap, so that both of them should result in the equivalent liq or ice states going to 0 within roundoff. To do that, I needed to move the partitioning of the total flux into liquid and solid to earlier in the subroutine and then recalculate those partitioning fluxes in conditions where we're applying an evaporation constraint. Note that I applied a max of 0 to the new fluxes because many initial conditions files have roundoff-level negative H2OSOI_LIQ, so without this limit, we were getting roundoff-level negative fluxes. Resolves ESCOMP#1979
Updates needed for pFUnit 4 and other externals updates (1) Lots of small changes needed for the update to pFUnit4. Note that this is a backwards-incompatible update, so we will require pFUnit 4 moving forward. (2) Externals updates: some of these are needed for the update to pFUnit 4; others are included to update externals to those in a recent CESM alpha tag. Update cdeps external to version with NEON.PRISM changes Conflicts: Externals.cfg
Sorry to be out of town and then nagging on a Friday afternoon. It looks like this PR is just waiting for final testing, but it's not high on the list of upcoming tags. Is this going to be able to come to main before too long? |
Hi @wwieder , I think @ekluzek was doing some testing, and it looks like that has been fairly finalized. That said, I'm not exactly sure of the status and whether or not if we have any particular roadblocks at the moment. @ekluzek if I can help with anything at this point, please let me know! By the way, those final tag updates also look good to me! |
@wwieder this is the next tag planned. I am combining it with a couple other unrelated PR's that are simple-bit-for-bit. I do view this as finalized now, especially as it sounds like @TeaganKing approves of the changes I made. |
Awesome, thanks @ekluzek ! That's great news! As far as using this in the tutorial, do we have a timeline for when those few PRs will likely be merged in? |
I think I can commit to by Monday. |
Ok, that sounds great! Thanks so much, @ekluzek ! |
Description of changes
This PR includes changes that allow PRISM precipitation to be used as a new datm stream.
Updates to
cime_config/config_component.xml
include additional valid values for PRECIP data stream names. Changes incime_config/usermods_dirs/NEON/defaults/user_nl_datm_streams
specify which input variables are gathered from which streams and specifies file location for PRISM data.Using PRISM precipitation instead of NEON precipitation does have a substantial impact on CTSM output (eg. latent heat flux biases).
Note:
This PR's functionality also depends on the PRISM PRECIP CDEPS PR, PRISM PRECIP ESMCI PR, and input data availability.
Contributors other than yourself, if any: @jedwards4b @wwieder @ekluzek
CTSM Issues Fixed: #1904
To Do:
cime_config/usermods_dirs/NEON/defaults/user_nl_datm_streams
accordingly). Also, I have some scripts for processing the data directly from PRISM; we should determine if we want to include these auxiliary scripts somewhere./CTSM/cime_config/usermods_dirs/NEON/NIWO/shell_commands
includes aDATM_YEAR_END
xmlchange
that prevented errors with NEON but is unnecessary with PRISM)