Move co2diag from 3hourly to daily for CPLHIST mode#402
Conversation
…e to the CO2 stream for a 24 hour avg file fixing ESCOMP#400
ekluzek
added
enhancement
…e is DEBUG on and off and one test with gnu
|
@billsacks I've run the aux_cdeps testing compared to cesm3_0_alpha08o and it looks good. Could you approve this so it gets merged in? |
|
It looks like I have permission to merge, so I'm going to go ahead and do that. Because my other work relies on this coming in. @billsacks there are a few things I'd like to chat with you about. And maybe @fischer-ncar if he's going to be the other main maintainer of CDEPS. With Jim gone, I'm not quite sure who are the main maintainers now... |
billsacks
left a comment
billsacks
left a comment
There was a problem hiding this comment.
@ekluzek - thanks a lot for fixing this issue for co2diag, and for adding these new tests. It's great to have this functionality tested!
I do have some comments on the new tests. It's too late to address them for this PR, but I'm opening them to address in a follow-up.
| @@ -0,0 +1,6 @@ | |||
| ./xmlchange DATM_CPLHIST_DIR="/glade/derecho/scratch/hannay/archive/b.e30_alpha08o.B1850C_MTso.ne30_t232_wgx3.330/cpl/hist/" | |||
There was a problem hiding this comment.
This needs to be moved into inputdata and shouldn't be pointing to scratch space.
| <test compset="1850_DATM%CRUJRA2024b_SLND_SICE_SOCN_SROF_SGLC_SWAV_SESP" grid="f10_f10_mt232" name="SMS_D_Ld5" testmods="datm/cplhist_create"> | ||
| <machines> | ||
| <machine name="derecho" compiler="intel" category="aux_cdeps"/> | ||
| </machines> | ||
| <options> | ||
| <option name="wallclock"> 00:10:00 </option> | ||
| <option name="comment">Make sure can run creating CPLHIST data to run DATM from with DEBUG on</option> | ||
| </options> | ||
| </test> | ||
| <test compset="1850_DATM%CRUJRA2024b_SLND_SICE_SOCN_SROF_SGLC_SWAV_SESP" grid="f10_f10_mt232" name="ERS_Ld5" testmods="datm/cplhist_create"> | ||
| <machines> | ||
| <machine name="derecho" compiler="intel" category="aux_cdeps"/> | ||
| </machines> | ||
| <options> | ||
| <option name="wallclock"> 00:10:00 </option> | ||
| <option name="comment">Make sure can run creating CPLHIST data to run DATM from with DEBUG off</option> | ||
| </options> | ||
| </test> |
There was a problem hiding this comment.
It feels like these tests for creating cplhist files belong in aux_cmeps, not aux_cdeps. They could still use datm, but they seem more about testing cmeps functionality, not cdeps functionality, so the time we'd want to run them is when making cmeps changes, not cdeps changes.
Also, I'd prefer to have a single test with debug mode on, to keep the test suite more trim and faster turnaround.
There was a problem hiding this comment.
@billsacks This does make sense. I'll move this over to cmeps in a future update. I assume you mean the tests should be in the cmeps repository and not cdeps? As an easier change I could change the testlist name here from aux_cdeps to aux_cmeps. Would the latter be acceptable?
What I think we really need though is a test that create CPLHIST files and then uses them to run CDEPS. Since, this kind of test uses both CMEPS and CDEPS, it wasn't clear to me where it would belong. This would also need to be a new SystemTest that does both steps to make sure it all works.
There was a problem hiding this comment.
I was thinking both that the tests belong in aux_cmeps and also that they belong in the CMEPS repository rather than the CDEPS repository. I don't think it makes sense to define aux_cmeps tests in CDEPS because then adjusting the CMEPS testlist requires a change in CDEPS, which feels like a pain.
Good point about this being a step towards a test that does both steps of generating the data and then running with it. And then, yeah, at that point it isn't clear to me where this test should be defined - but probably it should appear in both the aux_cmeps and aux_cdeps test lists... my initial thought is that the python file defining the test would live in one of those repositories (if there's one that it interacts with most in terms of settings, it could be best in that repo), and then there would be tests defined in both aux_cdeps (defined in the cdeps repo) and aux_cmeps (defined in the cmeps repo).
| <test compset="1850_DATM%CRUJRA2024_SLND_SICE_SOCN_SROF_SGLC_SWAV_SESP" grid="f10_f10_mg37" name="SMS_Ld5"> | ||
| <machines> | ||
| <machine name="derecho" compiler="intel" category="aux_cdeps"/> | ||
| <machine name="betzy" compiler="intel" category="aux_cdeps_noresm"/> |
There was a problem hiding this comment.
It looks like that was just a mistake. I did this in
and I don't see a mention of it, so I'll put it back.
4 participants
Description of changes
In order to synchronize with how the CPLHIST output files are handled in the latest CMEPS, this move the reading of co2diag from the 3-hourly file to the daily file. It also
Specific notes
Contributors other than yourself, if any: @billsacks
CDEPS Issues Fixed (include github issue #):
Fixes #400
Are there dependencies on other component PRs (if so list):
Needed for CDEPS to work with output using cmeps1.1.35
Are changes expected to change answers (bfb, different to roundoff, more substantial):
Answers for CPLHIST will be different and greater than roundoff, but it also didn't work previously
Any User Interface Changes (namelist or namelist defaults changes):
Add cplhist mode as an option to DATM_CO2_TSERIES
Remove co2diag from CPLHIST for the 3hourly stream file
When cplhist mode is used in DATM_CO2_TSERIES use the daily file
Testing performed (e.g. aux_cdeps, CESM prealpha, etc): So far running a case on Derecho_intel
Will run aux_cdeps
Hashes used for testing:
Definition of Done: