MIMICS-related refactor before introducing MIMICS #1340
Conversation
...thereby adding flexibility because as we add MIMICS to the list of decomp models (currently CN and BGC) we will need different values for these parameters
I ran these tests: ERP_D_Ld3_P36x2.f10_f10_mg37.I2000Clm50BgcCru.cheyenne_intel.clm-default -c /glade/p/cgd/tss/ctsm_baselines/ctsm5.1.dev033 --> PASS ERP_P36x2_D_Ld5.f10_f10_mg37.I2000Clm50Cn.cheyenne_intel.clm-default -c /glade/p/cgd/tss/ctsm_baselines/ctsm5.1.dev033 --> PASS The first (BGC test) defaults to use_vertsoilc = .true. and the second (CN test) defaults to use_vertsoilc = .false. so I repeated the second with use_vertsoilc = .true. with & without my code mods in *CascadeCN and both FAIL.
slevis-lmwg
added
PR status: work in progress
next
|
Next I will look into the params file refactor. Right off the bat I am aware of the following bfb mods:
|
Successfully repeated the two tests listed in this PRs intro. New params file clm50_params.c210418.nc currently located in /glade/scratch/slevis/temp_work/param_files In this file I also renamed k_frag -> k_frag_cn & tau_cwd -> tau_cwd_bgc
billsacks
removed
the
next
|
Linking this to #1356 where we plan to remove the "CN" option to BGC. @slevisconsulting don't plan on keeping the "CN" version around or putting any work into it. |
Thank you @ekluzek and did you resolve whether I should actually remove "CN" things in this PR or whether someone else should do it? Of course @wwieder has say in this decision, too :-) |
|
@slevisconsulting good question. I put the issue as a "next" so we'll discuss it next week at the Thursday Software meeting and likely decide. I think the important thing for you to do is to NOT put any work into the CN version of soil-BGC. Or at least the minimum amount possible. If it does get in your way in the next week -- let us know and we can talk about it sooner. Even a straight forward removal of the CN version is likely to end up in the peeling onion effect where you end up touching more files than you think you would. So it will take some work to do. So I think it's worth deciding who's going to do that. |
This commit covers roughly half of the refactor. This refactor should make the code more flexible to handle varying numbers of litter pools instead of expecting three. This should accommodate the introduction of an alternate DecompCascade model such as MIMICS that uses two litter pools.
Partly clean-up and partly getting the cheyenne test-suite to pass, except for Clm45Cn, Clm50Cn, and Clm45Fates tests. Maintaining CN is not a priority, but how about Clm45Fates? Also not a priority?
Thank you @ekluzek |
|
Cheyenne test suite OK. Expected tests fail (now this includes CN tests). Izumi test suite FAILed. Investigating 2 failures:
@ekluzek both failures are pgi. The same tests with gnu and intel passed on izumi. Similar tests (BgcCrop-crop*) passed on cheyenne. I reran the test w the larger rms diff, but w my last commit that used dev033: Also DIFF from baseline. Additional info about the two failing tests: What’s a reasonable path fwd in this case? |
|
The tests that @ekluzek recommended this morning PASS. Baseline tests: I will push some updates to the ChangeLog. Then the PR is ready for merge. As always pls let me know if I'm missing something. |
ekluzek
added
PR status: ready
|
@slevisconsulting I noticed you didn't enter the new paramfiles into svn inputdata. You should be able to do that looking at your permissions. It looks like when you ran on izumi, you copied the files over rather than letting them be populated from svn. I actually see running on izumi as a good way to catch if I forget to add files to inputdata. So I never copy by hand. So could you enter those into svn? I use the rimport script on cheyenne that's at ... /glade/p/cesmdata/cseg/inputdata/rimport Thanks. |
|
@slevisconsulting I also noticed that CN tests are breaking. Which is OK, since we view them as deprecated (see #1356). But, we should at least mark the CN tests as expected fails. And likely we should just remove those tests. And we are going to want to eventually remove the CN option as a possibility. I did wonder if keeping CN around would be good just to show that our infrastructure is flexible to support more than type of decomposition. But, I'm not sure about that as it would require you to put effort into something we plan to remove. |
|
@slevisconsulting it looks to me like some BGC and FATES tests are failing. Can you comment on that? ERP_D_Ld10.f19_g17.I1850Clm51BgcCrop.cheyenne_intel.clm-ADspinup |
|
Looks like everything is good on izumi, with just the CN test failing, but that's expected. |
|
I reran that list of unexpected failures and they passed the second time around. The second time I ran them as individual tests. I used -g to generate baselines for all these, so I think we should be fine. |
@ekluzek I ran rimport on the clm45 and clm50 files but will wait for your response on what to do about the ctsm51 file. |
@ekluzek and I agreed on the phone that:
|
@ekluzek gave me the go-ahead to run rimport on my ctsm51 file, as well. They're all done now. |
|
Note, we did notice an increase in memory based on some of the tests. There were some arrays that added a dimension to them, so I suppose that makes sense. |
Make certain history fields descriptive that have been labeled by number since #1340
I used existing infrastructure to add descriptive strings to certain history fields that I had labeled by number in ESCOMP#1340. While doing this, I applied the change to a bunch of other history fields that needed it. Some variable names for pools were also changed to use terms consistent with the new names as well.
I used existing infrastructure to add descriptive strings to certain history fields that I had labeled by number in ESCOMP#1340. While doing this, I applied the change to a bunch of other history fields that needed it. Some variable names for pools was also changed to use terms consistent with the new names as well.
I used existing infrastructure to add descriptive strings to certain history fields that I had labeled by number in ESCOMP#1340. While doing this, I applied the change to a bunch of other history fields that needed it. Some variable names for pools was also changed to use terms consistent with the new names as well.
Description of changes
Refactor aspects of the CTSM affecting our introduction of MIMICS before engaging in the actual introduction of MIMICS which has started in #1318 .
Specific notes
Introducing MIMICS initially appears to just require:
SoilBiogeochemDecompCascadeMIMICSModfollowingSoilBiogeochemDecompCascadeBGCModas a templateclm_varparandSoilBiogeochemDecompCascadeConTypeHowever, the hooks between the CTSM and MIMICS do not have 1-to-1 correspondence with the hooks between CTSM and BGC (or CTSM and CN). To avoid hacking things together just to make them work, we will try generalizing the hooks between the models.
Contributors other than yourself, if any:
At this stage @wwieder @ekluzek
CTSM Issues Fixed (include github issue #):
Note, #1356 becomes fully broken
Are answers expected to change (and if so in what way)? No bit-for-bit
Any User Interface Changes (namelist or namelist defaults changes)?
A couple of parameters that are currently set in the namelist will be moved to the params file.
Testing as of now:
PASS
ERP_D_Ld3_P36x2.f10_f10_mg37.I2000Clm50BgcCru.cheyenne_intel.clm-default -c /glade/p/cgd/tss/ctsm_baselines/ctsm5.1.dev033PASS
ERP_P36x2_D_Ld5.f10_f10_mg37.I2000Clm50Cn.cheyenne_intel.clm-default -c /glade/p/cgd/tss/ctsm_baselines/ctsm5.1.dev033The first (ie the BGC test) defaults to use_vertsoilc = .true. and the second (the CN test) defaults to use_vertsoilc = .false. so I repeated the second with use_vertsoilc = .true. with & without my code mods in
*CascadeCNand both FAIL. This tells me that the CN implementation was already broken with use_vertsoilc = .true.