-
Notifications
You must be signed in to change notification settings - Fork 322
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
Merge ctsm5.1.dev020 tag into fates_main_api #1257
Merge ctsm5.1.dev020 tag into fates_main_api #1257
Conversation
…m right now until cime allows drv_fld_in to be in ctsmconf directory
…ese are less important tests with Cru forcing
Urban mksurfdata update
Have a copy of btran2 that's just inside CNFire
clm5_1 physics starting point
Fang Li's latest Fire version - includes allowing clm5.1 phys version. New physics option is added called "clm5_1", with currently the new feature to use the latest fire changes. This has some adjustments to the fire model and includes some changes to the parameter file. Other new features will be added into clm5_1 in future tags. Also bring in mksurfdata changes for the raw urban dataset change. This adds some changes to mksurfdata for a new urban raw dataset, as well as preparation for new changes for some other urban changes that will be a future part of clm5_1. Also use the half degree lightning dataset by default for clm5_1. Start adding a new test list ctsm_sci that tests all the scientifically supported compsets. Some of those tests fail due to existing issues, that will be fixed later. Some more work done to change clm to ctsm, and allow for ctsm as a component.
Reduce duplication between caps Eliminate duplication of the "derived quantities for required fields" and corresponding error checking codes that repeat across the mct, nuopc, and lilac caps. This consolidates the code and reduces maintenance requirements. Resolves ESCOMP#918
I was getting a gridcell-level methane balance check error when running ERS_Lm25.1x1_smallvilleIA.IHistClm50BgcCropQianRsGs.cheyenne_gnu.clm-smallville_dynlakes_monthly. I'm thinking it was due to ESCOMP#43. So for now, I'm disabling methane in this testmod. (I thought about changing it to an Sp case, but (1) currently you're not allowed to run smallville in an Sp case, and (2) it would be nice to exercise as much of the BGC code as possible in this test, to exercise the dynamic landunit conservation code for BGC variables.)
…r on the same line
Dynlakes: fix some subtle issues ### Description of changes Fix a variety of subtle issues with dynamic lakes - particularly the accounting of total water and energy. ### Specific notes This branch contains the following commits: - a9fa875: This is needed to avoid counting lake water in the begwb and endwb terms, which is needed because these are used to calculate gridcell total water store (TWS), which in turn influences the methane code. Because the methane code was tuned around old values of TWS, changing TWS would lead to unintentional – and potentially large – changes in methane terms. Eventually we'd like to remove methane's dependence on TWS, but for now this workaround is needed to avoid changing behavior too much. See ESCOMP#659 (comment) for more details. - 52105c4: This minor fix is needed for the sake of water tracers / water isotopes. It shouldn't have any impact outside of that (because the tracer_ratio of bulk water is 1) - de3e12c and acf0984: This one is especially subtle; it is needed for backwards compatibility with old restart files. The main changes are in de3e12c; acf0984 is just a minor tweak on top of that. The problem is that, on existing initial conditions files, there can be already-existing DYNBAL_BASELINE variables (for LIQUID, ICE & HEAT). But these pre-existing variables will have baseline values of 0 for lake. Before this commit, when you started up from an old initial conditions file, the code would use these 0 values for lake baselines (because baselines are only reset if the user explicitly asks them to be reset with a namelist flag). This commit adds some code to detect if the initial conditions file is old, and if so, recomputes dynbal baselines for lake using the new definition. Note that some even older initial conditions files didn't have the DYNBAL_BASELINE variables at all; those would have been okay before this change: the problem is with initial conditions files that are somewhat but not very old - so have DYNBAL_BASELINE variables on them that use the old definition (where lake baseline values were 0). - 8088c3c: Minor fix for a pre-existing issue - a31875d: I'm not sure if this is actually needed, but I thought it would be good to group together the lake water content and the roughly equal-but-opposite baselines, so that these can cancel to near zero before adding the smaller terms. In principle, this should help maintain precision in these smaller terms. I thought this might help resolve some of the larger-than-expected answer changes I was seeing in testing, but I don't think it actually does... but I still thought this would be good to keep in place. **I have double and triple checked these changes, but it would be good to have an extra set of eyes on them to make sure I did this reordering correctly. In particular, I think there were some subtleties about when a term should accumulate on top of an existing value vs. setting the initial value of a variable.**
…t_veg, dlrad and ulrad, this is shown to be different on the PGI compiler, but should be only different by roundoff, since it's just a difference in order of operations for those terms
…d another additional test as well for one of the new compsets
… e-11 and e-12, e-13 showed problems
…n two tests to verify: SMS.f10_f10_musgs.I2000Clm50BgcCrop.izumi_pgi.clm-crop and SMS_Ld1_Mmpi-serial.f45_f45_mg37.I2000Clm50Sp.izumi_pgi.clm-ptsRLA
…ference of order roundoff
Small answer changes in preparation for adding option for bio-mass heat storage
This adds in some changes that may be roundoff different in preparation for bringing in the changes that allow turning on the bio-mass heat storage option. There's a few zeroed terms added to some calculations in CanopyFluxes. The new terms are identical to what will come in for BHS except the zeroed terms are not arrays here but scalars. I showed the new terms are only off by roundoff with a previous commit that added explicit tests for the terms and verified it for two tests.
…equired in the base class even though they aren't used in FATES versions, so they are empty and call endrun, if they were invoked wrongly
All non-fates testmods for the I need to manually run |
I should note that upon reviewing the DIFFs for the Fates testmods in the |
Manual run of
All other tests result in DIFFs. @rgknox I ran a 10 year Raw test results can be found here:
|
After reviewing the I reviewed the changelog and ran some additional quick (1 year) I duplicated my original notebook comparing the differences between dev020 and dev002 for all variables at the end of 10 years. The results appear to be b4b suggesting that all differences between fates_main_api and master up to dev020 and due wholly to dev002 : https://github.com/glemieux/fates-jupyter/blob/main/fates_main_api/fates_main_api-merges/ctsm5_1_dev002-f45.ipynb |
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.
It's hard to review these merge updates. But, it appears to be correct as far as I can see. Possibly the best check might be to compare this to the baseline version, and then compare that to the differences on the fates_main_api branch to it's baseline.
Description of changes
This PR brings the fates_main_api branch up to date with ctsm5.1.dev020 tag in preparation for merging fates_main_api into ctsm master.
Specific notes
The only major change that came up during the merge was the need to add
CNFireArea
,CNFireReadParams
, andCNFireFluxes
as empty functions to FatesFireBase.F90. This was necessary due to the refactor that took place in #1155 with e345c39.Contributors other than yourself, if any: @ekluzek
CTSM Issues Fixed (include github issue #): None
Are answers expected to change (and if so in what way)? Only fates-related answers
Any User Interface Changes (namelist or namelist defaults changes)? No
Testing performed, if any:
aux_clm
suite:/glade/u/home/glemieux/scratch/tests_0115-165815ch
fates
suite:/glade/u/home/glemieux/scratch/clmed-tests/fma-merge-ctsm5.1.dev020.fates-sci.1.43.2_api.14.2.0-ctsm5.1.dev020-Ce09dc512-F5534a940